Better typing for `renderPage`?

[magne4000]

First of all, thank you for this awesome lib (and really useful documentation)!

I was playing around with createPageRenderer (I'm using Typescript + Vercel + SolidJS) like so:

const renderPage = createPageRenderer(...);
...
const pageContext = await renderPage(pageContextInit);

Here, my pageContext should have custom fields that could be returned (like redirectTo as in your examples, or cookies, that is responsible to set Cookies on headers).

So I checked the renderPage typings:

Good, there is a PageContextAdded type that I can use:

const pageContext = await renderPage<MyCustomType, typeof pageContextInit>(pageContextInit);

This should have been enough I guess, but the right typing is not quite right IMO:

PageContextInit & (({ httpResponse: HttpResponse } & PageContextAdded) | { httpResponse: null })

Looking closer, PageContextAdded is added to the return type only if httpResponse is of type HttpResponse, which in both of my use cases (redirect and cookies) is not necessarily the case.

Perhaps I'm doing this wrong? But if not, here is the typing I propose:

PageContextInit & { httpResponse: HttpResponse | null } & PageContextAdded
// or with Partial but it could mess some type definitions
PageContextInit & { httpResponse: HttpResponse | null } & Partial<PageContextAdded>
// see https://www.typescriptlang.org/play?#code/C4TwDgpgBACghgcwgYQPYDtgQB7AIIAmBEBUAvFAN4BQUUcAXFOgK4C2ARhAE4A0tUDk1ace-AL5QAPlSgBjYey7co4gNzVqoSFAAa5WHG7AAlnAA2AHnhI0mHPiIkAfBupyMAZ2BRsTfRSU9EwAjLyCTABMqhoe6N5QIEw2KBhYuITEpIHBUGERUNHqmiYAZlAAFNgAdHAAlFQCdAD0zVAAogBKnQDynU3yXj5CzEo8BjUcGuIl5RUgtQ00dC1tAHKoUDzcqCpwnlvYkHJYBANxCSMiygYLU9TiQA

[brillout]

How about PageContextInit & (({ httpResponse: HttpResponse } & PageContextAdded) | ({ httpResponse: null } & Partial<PageContextAdded>))?

If an error throws early then PageContextAdded === {} so we need Partial.

which in both of my use cases (redirect and cookies) is not necessarily the case.

Can you elaborate the cookies case?

[brillout]

Can you elaborate the cookies case?

I'm curious.

[magne4000]

How about PageContextInit & (({ httpResponse: HttpResponse } & PageContextAdded) | ({ httpResponse: null } & Partial<PageContextAdded>))?

If an error throws early then PageContextAdded === {} so we need Partial.

👍

Can you elaborate the cookies case?

I'm currently using onBeforeRender server side to store Cookies (JWT encoded) to save some small state that represents a step or multiple steps in a user flow, e.g.:

1. Login to Platform X with username and password
2. Credentials are sent via onBeforeRender to Platform X
3. Platform X sends back an event telling that 2FA is needed. That is where I need to store username and password for the user so he/she doesn't have to send them again
4. Ask user 2FA code to user
5. Decode JWT Cookie to retrieve username and password, and 2FA from post request -> I can send request again to Platform X

[magne4000]

How about PageContextInit & (({ httpResponse: HttpResponse } & PageContextAdded) | ({ httpResponse: null } & Partial<PageContextAdded>))?

If an error throws early then PageContextAdded === {} so we need Partial.

PageContextInit & (({ httpResponse: HttpResponse } & PageContextAdded) | ({ httpResponse: null } & (PageContextAdded | {}))) would perhaps be more accurate.

[brillout]

PageContextInit & (({ httpResponse: HttpResponse } & PageContextAdded) | ({ httpResponse: null } & (PageContextAdded | {}))) would perhaps be more accurate then

No because the error can also be thrown between two hooks. So the first hook succesfully added his additional pageContext but not the second.

Objections? Let me know if not and I'll release the new type.

[magne4000]

Hum indeed, but if a hook sets {a: number, b: number} and another one {c: number}, we lose accuracy in the definition. Partial will force you to check a and b separately, but in the mean time it also doesn't force the developer to be aware of internal mecanism when he defines its types. Using Partial is safer in most cases IMO 👍 . I'll see if I come with another workaround to have better typing if the developer know what he/she's doing.

[brillout]

The best would be https://github.com/brillout/vite-plugin-ssr/issues/176 so that all types are automatically stiched together for the user.

[brillout]

You're right we loose accuracy but Partial<PageContextAddendum> is still more accurate than {}.

Let me release the new type. Last chance to object :-)

[magne4000]

LGTM

[brillout]

Released in v0.3.20.