Improve hooks?

[brillout]

• How about deprecating onBeforeMount() in favor of onCreateApp()? AFAICT the onBeforeMount() hooks of vike-{pinia,vue-query} can be merged into onCreateApp().
• How about deprecating onAfterRenderSSRApp() in favor of a new hook onAfterRenderHtml() that is called right before escapeInject? Users could use !!pageContext.Page to check whether the app is SSR'ing. Also, a neat thing here is that the new name would then align with vike-react's hooks.

[pdanpdan]

How about deprecating onBeforeMount() in favor of onCreateApp()? AFAICT the onBeforeMount() hooks of vike-{pinia,vue-query} can be merged into onCreateApp()

In Vue onBeforeMount and onMounted guarantee that the code will only run on client side, while onCreateApp is run both on server and on client. So in order to merge onBeforeMount and onCreateApp a new parameter to specify where it is run would be needed, making the code in the hook more complex. And even with that parameter it would be impossible to make sure that the code that should run on create in all hooks will be run before the code that should run on before mount in all hooks is run. So I would say it would not be a good move. A simple example is vike-pinia where the hydration should run only on client side.

How about deprecating onAfterRenderSSRApp() in favor of a new hook onAfterRenderHtml() that is called right before escapeInject? Users could use !!pageContext.Page to check whether the app is SSR'ing. Also, a neat thing here is that the new name would then align with vike-react's hooks.

The check for when it is run on SSR is possible, but the ctxWithApp would not be available when not SSR'ing. BTW, I could not find a onAfterRenderHtml or something with a similar meaning in vike-react.

[brillout]

The check for when it is run on SSR is possible, but the ctxWithApp would not be available when not SSR'ing.

Indeed, the user would then have to use pageContext.app! instead of pageContext.app, i.e. tell TypeScript that it exists.

I think that's the only added value of onAfterRenderSSRApp()? I'm inclined to think it isn't worth it to create an entire new hook just for that?

BTW, I could not find a onAfterRenderHtml or something with a similar meaning in vike-react.

Indeed vike-react doesn't have it as of today, but I can see use cases for it so I think vike-react should/will eventually define it.

As for the naming, vike-react already has on{Before,After}RenderClient(). So maybe there is an opportunity to follow that naming convention. It also aligns well with the Vike core hooks onRender{Client,Html}() and onBeforeRender().

In Vue onBeforeMount and onMounted guarantee that the code will only run on client side, while onCreateApp is run both on server and on client. So in order to merge onBeforeMount and onCreateApp a new parameter to specify where it is run would be needed, making the code in the hook more complex. And even with that parameter it would be impossible to make sure that the code that should run on create in all hooks will be run before the code that should run on before mount in all hooks is run. So I would say it would not be a good move. A simple example is vike-pinia where the hydration should run only on client side.

Good point. Let me think about this.

[4350pChris]

Imo onBeforeRenderClient wouldn't be fitting - as we're not calling it before the onRenderClient hook but within, right before app.mount(container).

[brillout]

How about we rename onBeforeMount() to onBeforeRenderClient()? Exactly the same but the only exception is that we also call it right before changePage() (the user can use pageContext.isHydration).

See also https://github.com/vikejs/vike-react/issues/110#issuecomment-2094056007 which is already implemented.

Thoughts?

[brillout]

Imo onBeforeRenderClient wouldn't be fitting - as we're not calling it before the onRenderClient hook but within, right before app.mount(container).

Yea, I also thought about that. But I'd argue it's called before Vue rendering, not before vike-vue rendering.

I agree it's a bit misleading, but I'm thinking the pros outweights the cons.

[brillout]

If we end up with onAfterRenderHtml() + onBeforeRenderClient() then onCreateApp() is the only vike-vue specific hook.

But, yea, I do agree that onBeforeMount() is a clearer than onBeforeRenderClient().

I ain't sure, I'll think about it.

[brillout]

Actually, how about onBeforeHydration()? I think it's both clear and can be aligned with vike-react.

[4350pChris]

So, no calling it before changePage? That might have some use cases but I'm not sure.

[4350pChris]

I think onAfterRenderHtml is fine. Even though it's technically not called after but, again, from within. However, looking at it like onRenderHtml is the function that outputs your HTML (which is my perception of it anyways) it is semantically correct imo.

[brillout]

So, no calling it before changePage? That might have some use cases but I'm not sure.

Yea, that's the trade off. Not sure either.

[brillout]

I think onAfterRenderHtml is fine. Even though it's technically not called after but, again, from within.

Agreed, although conceptually it's equivalent. onBeforeRenderHtml is but not onAfterRenderHtml.

[brillout]

I think onAfterRenderHtml is fine. Even though it's technically not called after but, again, from within.

It's actually the same argument than onBeforeRenderClient(): it's after the HTML rendering from Vue's perspective, not from the global HTML perspective.

But, yea, it's a bit misleading.

[4350pChris]

Honestly I feel like it's ok to align it with vike-react even if the semantics aren't perfect. And your explanation of looking at it from vue's perspective makes sense. Best case is the user can just use vike and largely forget about it. So let's not clutter that by introducing vike / SSR terminology.

[brillout]

I guess the only question left is: do we go for onBeforeRenderClient() or onBeforeHydration()? I think I've a slight preference for onBeforeRenderClient(), but I'll think more about it.

@pdanpdan WDYT? Also about the rest.

Best case is the user can just use vike and largely forget about it.

Yea, the long-term term goal is that most users just use Vike extensions.

[brillout]

let's not clutter that by introducing vike

That's a good point, actually. Most users won't know about onRender{Client,Html}(). So they'll actually think from Vue's perspective, not Vike's perspective.

[pdanpdan]

I'm not very sure about the implications.

• onBeforeRenderClient will replace onBeforeMountApp and be called also after client side navigation
• mounting is a well defined lifecycle state in Vue, while client rendering is not very clear as meaning
• for me it is easier to use multiple clearly defined lifecycle hooks that using fewer but each with branching inside based on is on server/client, is hydration or not, ...

But I don't have a clear image right now about what would be the advantages/disadvantages, so you can ignore my opinion for the moment.

[brillout]

Makes sense. Although I'd still prefer the name onBeforeHydration() because I think that "hydration" is a more widespread term than "mount".

• for me it is easier to use multiple clearly defined lifecycle hooks that using fewer but each with branching inside based on is on server/client, is hydration or not, ...

I share the sentiment, but on the other hand having too many hooks can backfire. A big list of hooks would make it significantly slower to 1. digest and 2. experiment with each potentially relevant hook. If you have only on{Before,After}RenderClient() and onCreateApp() then it's going to be mostly clearer which want you'll want to try. If you have 5 client-side hooks, it becomes annoying to try & guess which one is the right one. I agree the name on{Before,After}RenderClient() is far from ideal but I'm thinking the pros outweighs the cons.

Personally, all-in-all, I prefer onBeforeRenderClient(). But we can continue the discussion and dig if you feel that it'd be worth it. Otherwise, let's proceed with https://github.com/vikejs/vike-vue/pull/96.

[brillout]

Some slightly better names :sweat_smile: but let's stick to the current ones and re-consider the following ones before releasing v1.0.0.

• onRenderHtmlStart()
• onRenderHtmlEnd()
• onRenderClientStart()
• onRenderClientlEnd()

Or maybe onRender{Client,Html}{Begin,Finish}.

[pdanpdan]

I would rather go with on{Before,After}Render{Html,Client}

[brillout]

:+1:

Also we'll probably have plenty of new insights before we release v1.0.0, so let's see what we feel like by then.