Rename `contextProps` to `pageContext`. about vike

[deckchairlabs]

I'm not sure if I've mentioned the below before on a different thread, apologies if I have.

This may be out of scope, but I've been thinking if the naming, especially for the server API, was more focused around what stage the render cycle was in.

// _default.server.page.ts
// Could redirect
export async function onRequest() {
  // Send a redirect perhaps, unauthorized etc?
  // Return nothing just continues with render?
}

export async function onResponse(renderResult: string, originalRequest: Request) {
  // Could modify the HTML perhaps?
  return renderResult.
}

export async function onBeforeRender({ contextProps, Page }) {
    return {
        contextProps: {
            ...contextProps,
            foo: 'bar'
        }
    }
}

[brillout]

@chrisvariety Yes the idea is to rename all contextProps variables.

For example, renaming all contextProps to renderContext while also renaming addContextProps to onBeforeRender.

export async function onBeforeRender({ renderContext, Page }) {
    return {
        // Add stuff to `renderContext`
        renderContext: {
            foo: 'bar'
        }
    }
}

export async function render({ renderContext, Page }) {
   // ...
}

[brillout]

I'm leaning towards renaming addContextProps to addPageContext.

I do like the onBeforeRender idea, but it's sightly misleading for people who use useClientRouter() (with Client-side Routing, when the user navigates to a new page, the browser makes a server request and onBeforeRender() is called while the render() hook is not called: the new page is only rendered to the DOM and is not rendered to HTML).

Happy to be contradicted and discuss.

[brillout]

@deckchairlabs Because it will conflict with other context objects, e.g. https://github.com/brillout/wildcard-api. Imagine a framework built on top of vite-plugin-ssr and Wildcard API; it will be confusing to have several context objects that mean different things.

[brillout]

Got a new idea, I'm thinking to name it renderContext.

The thing is that contextProps lives only during rendering; the contextProps object is created when a page rendering starts (it's actually the user that creates the object when he does await renderPage({ url, contextProps: {} }) and contextProps is (usually) free for garbage collection after the page rendering is done.

renderContext may sound weird at first but it might actually clarify things. I'll meditate about it.

[deckchairlabs]

I see, how about something more event/hook based? onBeforeRender

// _default.server.page.ts
export async function onBeforeRender({ contextProps, Page }) {
    return {
        contextProps: {
            ...contextProps,
            foo: 'bar'
        }
    }
}

[brillout]

Interesting, I didn't think about this.

[brillout]

@deckchairlabs I like your idea. I'll medidate about renaming addContextProps to onBeforeRender. And good point that it will make it natural to add further hooks shall the need arise.

[CanRau]

Think I'm fine with all the suggesting, thanks for pinging and asking 🙌

🤜🤛

[brillout]

I'm going to rename contextProps to pageContext and addContextProps to addPageContext in the next couple of days.

Done and released in 0.1.0-beta.45.

Stage hooks are still open for consideration at #71.

[brillout]

This is the last breaking change before I release the first non-beta version.

I will provide scripts to ease migration, e.g. grep addContextProps | xargs sed -i 's/addContextProps/addSsrContext/g'.

Let me know if you object with the new name ssrContextpageContext.

EDIT: Changed title from "Rename contextProps to ssrContext." to "Rename contextProps to pageContext.".

@deckchairlabs
@doeixd
@gryphonmyers
@chrisvariety

(@gryphonmyers I will take care of updating the PR and resolve merge confits.)

[brillout]

cc @ozergul @alsyapukov @aral @CanRau @ruslankonev @naranjamecanica

[rulentor]

Hi!
doc here (https://github.com/brillout/vite-plugin-ssr) not true for boilerplate template.
How me get data from (https://swapi.dev/api/films/${movieId}/?format=json) by node-fetch and out Page as json. How many times knocking result only empty {} in browser (link /movies/1) How knew clearing mind truth? Big fans before

[brillout]

@rulentor write me on Discord

[deckchairlabs]

Why can't it just be simply context?

[chrisvariety]

addContextProps is just half of the story though, what are we calling it in function render({ Page, contextProps }: { Page: ReactComponent; contextProps: ContextProps }) { ... } ? It seems a little weird to rename to onBeforeRender but still have the render function have contextProps.

[gryphonmyers]

This all sounds good, however I'm now even more confused by the prospect of doing client-side fetching as described in #52. If it's now renderContext, how / why does the client have the ability to affect it when it's the server that does the rendering?

[brillout]

@gryphonmyers

I'm still pondering the renaming.

So far I like the renderContext name as it makes it clearer that the renderContext object lives only for the purpose of rendering.

I'm hesitating about onBeforeRender precisely because, when using useClientRouter() and when the user navigates to a new page, then it's somewhat confusing that the only thing called on the server is onBeforeRender.

[brillout]

From discord https://discord.com/channels/815937377888632913/815937377888632916/842824951350427688

pageContext and getPageContext is what came to my mind. Since it is all about the page (no?).

My reply:

I like your idea, makes sense. It somewhat conflicts with pageProps but we can rename that as well.

[brillout]

My current favorite is now pageContext.

[brillout]

I decided for pageContext.

I'm still hesitating about the hook thing.

[brillout]

I'm going to rename contextProps to pageContext and addContextProps to addPageContext in the next couple of days.

I'm still thinking about the stage-hook thing. I like it but I still need time to let the idea sink in. If anyone as more thoughts about stage-hooks, now is the time.

Looking forward to see what we come up with. Well designed stage-hooks could be a wonderful thing.

Opened a new ticket for it #71.

[gryphonmyers]

I'm going to rename contextProps to pageContext and addContextProps to addPageContext in the next couple of days.

I'm still thinking about the stage-hook thing. I like it but I still need time to let the idea sink in. If anyone as more thoughts about stage-hooks, now is the time.

Looking forward to see what we come up with. Well designed stage-hooks could be a wonderful thing.

Opened a new ticket for it #71.

I guess my main thought there is if the stage hooks are under consideration and hypothetically do get implemented, would addPageContext be better suited as an on* hook for consistency?

[brillout]

Yes, once/if we go with stage-hooks, the plan is to then deprecate addPageContext.

[brillout]

Every hook is now a stage hook. I.e. addPageContext() is now onBeforeRender().

Thanks for all the lovely feedback.