feat: on boot draft #1679
feat: on boot draft #1679
Conversation
| const globalObject = getGlobalObject<{ | ||
| onBootResult?: Record<string, unknown>, |
There was a problem hiding this comment.
Yes, this goes in the right direction: we want to call onBoot() only once and cache its result "forever".
| } | ||
|
|
||
| if (isObject(globalObject.onBootResult)) { | ||
| pageContext = { |
There was a problem hiding this comment.
I think we can use Object.assign() instead of re-creating a new object.
| @@ -206,6 +207,9 @@ const configDefinitionsBuiltInGlobal: Record<ConfigNameGlobal, ConfigDefinitionI | |||
| onBeforeRoute: { | |||
| env: { server: true, client: 'if-client-routing', eager: true } | |||
| }, | |||
| onBoot: { | |||
| env: { server: true, client: true, eager: true } | |||
There was a problem hiding this comment.
I'm not sure, but I don't think we need eager.
Actually, we need eager if we make onBoot() non-global and cumulative (see my sibling comment).
| * | ||
| * https://vike.dev/onBoot | ||
| */ | ||
| type OnBoot = ( |
There was a problem hiding this comment.
Let's name it OnBootAsync to keep it consistent with the other type names.
| return { onBeforeRouteHook: hook, filesystemRoots: null } | ||
| const onBootHook = getHookFromPageConfigGlobal(pageConfigGlobal, 'onBoot') | ||
| const onBeforeRouteHook = getHookFromPageConfigGlobal(pageConfigGlobal, 'onBeforeRoute') | ||
| return { onBootHook, onBeforeRouteHook, filesystemRoots: null } |
There was a problem hiding this comment.
It's a good question whether onBoot() should be global.
Maybe we can make onBoot() non-global and cumulative. In other words: the result of the various onBoot() hooks are cached and only the results of the relevant onBoot() hooks are applied each time Vike creates a new pageContext object.
But this means that Ah, no, actually if make onBoot() can be called later upon navigating to a new page and onBoot() is a misnomer then.onBoot() eager then we can call all onBoot() hooks at boot time.
There was a problem hiding this comment.
I'm actually leaning more towards making onBoot() global as you suggested.
The thing is that, if we don't make it global, then we have to resolve routing before calling onBoot(), which is somewhat contradictory as an idea of onBoot() is to call it as early as possible and before any request is made to the server.
If a user wants a per-page hook, then he can use onBeforeRender(). (So far I don't think we need to introduce a second hook onRequest().)
WDYT?
| @@ -149,6 +149,14 @@ async function renderPageAndPrepare( | |||
| // From now on, renderContext.pageConfigs contains all the configuration data; getVikeConfig() isn't called anymore for this request | |||
| } | |||
|
|
|||
| if (renderContext.onBootHook) { | |||
| const onBootResult = await renderContext.onBootHook?.hookFn(pageContextInit) as Record<string, unknown> | |||
There was a problem hiding this comment.
I don't think we should call onBoot() at request-time? My thinking was to call onBoot() exactly once and as soon as possible whenever its code is loaded and then cache its result.
|
Hi Olavi, thank you for opening this PR! It goes in the right direction. I've left a couple of comments. I'm a bit busy with two Can't wait to land |
No description provided.