Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client-only guard() hook for pre-rendered pages #1916

Open
lfos opened this issue Oct 8, 2024 · 15 comments
Open

Client-only guard() hook for pre-rendered pages #1916

lfos opened this issue Oct 8, 2024 · 15 comments
Labels

Comments

@lfos
Copy link

lfos commented Oct 8, 2024

Description

To reproduce, add a simple guard() hook and configure guard() to run client-side only:

export async function guard(pageContext) {
  console.log("guard");
}

The hook is only executed when using client-side navigation, and not on initial page load.

After reading the first few comments in #1600, I thought this was a known limitation, but after reading the whole discussion and re-reading https://vike.dev/guard, I am no longer sure this is known/intended behavior. (If it is, I believe the documentation should be further improved.)

@lfos lfos added the bug 💥 label Oct 8, 2024
@lfos
Copy link
Author

lfos commented Oct 8, 2024

For context, this is on an SSG site, using (or attempting to use) the hook to redirect/render different pages under certain preconditions.

@brillout brillout added enhancement ✨ New feature or request and removed bug 💥 labels Oct 8, 2024
@brillout
Copy link
Member

brillout commented Oct 8, 2024

under certain preconditions

Can you elaborate on that?

@lfos
Copy link
Author

lfos commented Oct 8, 2024

under certain preconditions

Can you elaborate on that?

Authentication with SSG is an example that was already mentioned in #1600, but there are other similar scenarios, such as having a whole part/section of a website that is only available after the user has taken specific action (e.g., added some data, made a purchase, ...) Essentially anything you might imagine as reasonable use cases for guard() on a SSR page.

@lfos
Copy link
Author

lfos commented Oct 9, 2024

@brillout If this is treated as "enhancement" rather than a bug, would it be possible to at least make the documentation very clear about the current state in the meantime? For example, https://vike.dev/guard#environment mentions that guard() can be configured to run on the client side, and the section on hook execution order here seems to explicitly state that client-side configuration means the hooks are executed on both both initial render and navigation. It took me quite some time and manual debugging to understand the current state/limitations and inconsistency with documentation.

@brillout
Copy link
Member

brillout commented Oct 10, 2024

IIRC the guard() hook is actually called, but at pre-render time. It indeed doesn't suit authentication use cases. I'll think about this once I'm done with my current priorities (ETA tomorrow / this week).

@brillout brillout changed the title Client-only guard() hook not executed on initial page load Client-only guard() hook for pre-rendered pages Oct 12, 2024
@brillout
Copy link
Member

brillout commented Oct 12, 2024

I agree that for pre-rendered pages guard() should be called only on the client-side. I'm labeling this as highest-prio since it's a blocker for users (there is a workaround so it actually isn't a blocker, but we should still make guard() work as users expect).

The Vike's CLI (#1340 and #1341) is a blocker for implementing this, so I'm afraid this will take a little while until I can proceed with the implementation.

After I'm done with my work on improving Vike's website and docs, I'll be working on these two blocking tickets and then finally this ticket.

@brillout
Copy link
Member

In the meantime, you can try to use throw render() / throw redirect() in one of the many client-side hooks such as onHydrationEnd() and/or onBeforeRenderClient().

@lfos
Copy link
Author

lfos commented Oct 12, 2024

I agree that for pre-rendered pages guard() should be called only on the client-side. I'm labeling this as highest-prio since there is a clear design decision and it's a blocker for users (there could be a workaround so maybe it isn't a blocker).

Great, thank you!

In the meantime, you can try to use throw render() / throw redirect() in one of the many client-side hooks such as onHydrationEnd() and/or onBeforeRenderClient().

I don't think either of these work, unfortunately. onBeforeRenderClient() seems to have the same problem as guard(): it is not called client-side on the first page load. When throwing render() or redirect() from onHydrationEnd() , nothing happens; maybe it is too late at that point?

@brillout
Copy link
Member

I'll have a look at why it doesn't work. ETA this week.

@brillout
Copy link
Member

(New ETA this week.)

@brillout
Copy link
Member

brillout commented Oct 27, 2024

onBeforeRenderClient() seems to have the same problem as guard(): it is not called client-side on the first page load.

No, onRenderClient() (and thus onBeforeRenderClient()) is always called including for the first page.

When throwing render() or redirect() from onHydrationEnd() , nothing happens

I just tried calling throw redirect() in onHydrationEnd() and it's working. Reproduction welcome.

I also tried inside onBeforeRenderClient() and it's working as well. Reproduction for this welcome as well.

@lfos
Copy link
Author

lfos commented Oct 28, 2024

onBeforeRenderClient() seems to have the same problem as guard(): it is not called client-side on the first page load.

No, onRenderClient() (and thus onBeforeRenderClient()) is always called including for the first page.

When throwing render() or redirect() from onHydrationEnd() , nothing happens

I just tried calling throw redirect() in onHydrationEnd() and it's working. Reproduction welcome.

I also tried inside onBeforeRenderClient() and it's working as well. Reproduction for this welcome as well.

Thanks for investigating! Did you also test this with SSG pages generated with npm run build/npx vite build (and not just npm run dev/npx vite)? If you can't reproduce, I'll try to put together a minimal example over the coming days.

@brillout
Copy link
Member

brillout commented Oct 28, 2024 via email

@lfos
Copy link
Author

lfos commented Nov 1, 2024

It looks like you're right; I had an independent bug in my code. After fixing it, onBeforeRenderClient() is triggered correctly.

The last part that's missing for this to be a viable workaround is a way to set pageContext. While onBeforeRenderClient() seems to differ from onBeforeRender() in that we cannot return an (updated) page context (why?), it seems to be possible to just overwrite/add properties directly. Can you confirm this is supported, in the sense that there shouldn't be any unexpected side effects? Thanks again for all the help!

@brillout
Copy link
Member

brillout commented Nov 1, 2024

Yes, see https://vike.dev/pageContext#custom and e5dc5c4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants
@brillout @lfos and others