feat(react-router): add SerializesTo brand type

[phryneas]

Apart from the turbo-stream support for ReadableStream, this is the only other change that we would need to add support for preloading Apollo Client data during SSR in loader functions.

The problem here is that Apollo Client uses a branded type QueryRef<TData, TVariables> that looks like this:

export interface QueryRef<TData = unknown, TVariables = unknown> {
  /** @internal */
  [QUERY_REF_BRAND]?(variables: TVariables): TData;
}

We are using an "imaginary" function here to preserve variance when passing around the type - TVariables should be contravariant, so it is referenced as a method argument, while TData is covariant, so it is passed as a method return value.

This function doesn't really exist on the transport object and no other properties on it are typed, since the user should never interact with the object apart from passing it into a consuming useReadQuery(queryRef) hook in the browser.

Unfortunately, currently that QueryRef above is turned into { [QUERY_REF_BRAND]?: undefined }, since Serialize removes all functions.

This SerializesTo type would be an escape hatch to allow it for e.g. library authors to pass around branded values like this by specifying what the "serialized" result type should be.

So the preloadQuery function that we provide for loaders would return something of the type SerializesTo<QueryRef<TData, TVariables>>.

[changeset-bot]

🦋 Changeset detected

Latest commit: 938d3d9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
react-router	Major
@react-router/architect	Major
@react-router/cloudflare	Major
@react-router/dev	Major
react-router-dom	Major
@react-router/express	Major
@react-router/node	Major
@react-router/serve	Major
@react-router/fs-routes	Major
@react-router/remix-routes-option-adapter	Major
create-react-router	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[remix-cla-bot]

Hi @phryneas,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[phryneas]

Okay, seems like a lot of things moved in the last two weeks, did some merging and reorganizing.

Verified with a local build in apollographql/apollo-client-nextjs@2b56da6 (#394) that the type still works as expected.

[phryneas]

At this time, this feature is ready on the Apollo side, so I would ask you to please give this a review, or at least some feedback, as this type change is the "missing piece" for us.

The whole PR can be seen here: apollographql/apollo-client-nextjs#394
This would be used in userland like this: https://github.com/apollographql/apollo-client-nextjs/blob/pr/react-router-7/integration-test/react-router/app/routes/home.tsx
You can see this "live" here: https://apolloc-git-ab15ba-apollo-client-next-package-integration-tests.vercel.app/ - a multipart graphql request is started during SSR in a loader, and both in SSR and in the browser, the results are merged into a normalized cache. You can see multiple chunks coming in slightly delayed, and as it is a client-side normalized cache, it can be live manipulated and refetched on the client without another server roundtrip (but navigation can merge more loader data in in the future).

Addressed

[phryneas]

@pcattori is there anything I can do from my side to help this move forward? Are there any open questions that you need answered?

[pcattori]

@phryneas I was expecting $__RR_SerializesTo?: T without the wrapping []. Does wrapping in [] come with any benefits over unwrapped T?

[phryneas]

It's probably not a big deal, but It's slightly more safe around undefined:

{
  interface WithoutWrapping<T> {
    foo?: T;
  }

  type SerializesTo<T> = T extends WithoutWrapping<infer F> ? F : "other";

  type Test1 = SerializesTo<WithoutWrapping<number>>;
  //    ^?  type Test1 = number

  type Test2 = SerializesTo<WithoutWrapping<number|undefined>>;
  //    ^?  type Test2 = number | undefined

  // this could happen if for some reason the TS compiler inlines the `WithoutWrapping` type
  // when creating declarations for a third-party-package
  // an explicitly stated `undefined` would get lost
  type Test3 = SerializesTo<{ foo?: number|undefined }>;
  //    ^?  type Test3 = number
}

[pcattori]

Could you explain why the brand is optional in the first place?

Why not have:

export type SerializesTo<T> = {
  __ReactRouter_SerializesTo: T;
}

[phryneas]

For assignability reasons - if it were non-optional you'd need an as any or as unknown when using it (example is obviously oversimplified):

type SerializesTo<T> = {
  __ReactRouter_SerializesTo?: [T];
};


const foo: number & SerializesTo<string> = 5

type SerializesToNonOptional<T> = {
  __ReactRouter_SerializesTo: [T];
};

// this errors
const bar: number & SerializesToNonOptional<string> = 5

[phryneas]

Also, just thinking of this - this is not actually there at runtime, so it keeps the type kinda "correct" in line with what you'd expect at runtime.

[pcattori]

Is the $__ prefix a widely used convention? I was thinking of __ReactRouter_ prefix instead of $__RR_ just for increased clarity, but wasn't sure if there were conventions I'm unaware of

[phryneas]

I'm perfectly fine with __ReactRouter_SerializesTo, too - I just hadn't seen any precedence and wanted to be extra sure :)

We use the $ (even with a space) in GraphQL because it's not allowed per spec for field names and we avoid collisions, but here it's not really necessary.

I'll fix this up to be __ReactRouter_SerializesTo later!

[phryneas]

Okay, made that change!