apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.38k stars 2.66k forks source link

Re-queries on hydration after ssr using renderToPipeableStream #11319

Open iamafansev opened 1 year ago

iamafansev commented 1 year ago

Issue Description

version: 3.8.* To begin with, it is better to consider the problem using an example: Example with useSuspenseQuery & renderToPipeableStream https://github.com/iamafansev/test-suspense-query

In this example, the fetchPolicy is set to cache-and-network. We expect that there will be no duplicate requests during hydration since we have a valid cache.

However, we see repeated outgoing requests in the developer tools.

At the same time, if you set the caching policy to cache-first, then repeated requests are not executed. This confirms that the cache contains correct data and the request need not be executed.

When I encountered this problem, I thought that the problem was only when using new features such as useSuspenseQuery, but I have another example: Example with useQuery / getDataFromTree & renderToPipeableStream (workaround to use React.(Suspense / lazy)) https://github.com/iamafansev/use-query-with-lazy-ssr

There is some workaround here to use pause rendering just to wait for lazy components. In the server-side implementation, we first traverse the tree using getDataFromTree and only after that, we pass the tree to renderToPipeableStream.

Similar to the first example, a repeated request is performed here with the cache-and-network caching policy and is not performed with cache-first. At the same time, if you do not use renderToPipeableStream for the example with useQuery, then repeated requests will not be executed under the cache-and-network caching policy. The problem also appears in version 3.7.17

Link to Reproduction

https://github.com/iamafansev/test-suspense-query

Reproduction Steps

phryneas commented 1 year ago

Hi there, lots of things to unpack here!

Unfortunately, you've fallen into a big pitfall the React team hasn't really documented well: there is currently no data fetching story for renderToPipeableStream in React.
See twitter discussion here (I'm really sorry that I have to give twitter as a reference here - I'd love to link you some docs, but this just hasn't been documented at all 🤦).

You can do the workaround you are currently doing - running getDataFromTree, gather all the data you need and then run renderToPipeableStream - but unless I miss some nuance here, there isn't really anything you gain from it: you essentially fully block the render, and then start a stream that could at that point in time just as well be a synchronous render (after all, you did all the async work before!).
It's also not really something we support, since - see above - it doesn't seem like it really makes sense to use renderToPipeableStream under these circumstances.

I have recently talked with Josh Story about this, and there is a good chance that React 19 might include a few helpers that would be necessary for us to get this working - but as of right now, there is just no React API for us to inject into the stream, so we cannot support this.
We do work around that and do have support for renderToPipeableStream in our Next.js helper library, but that relies on Next.js internals that kinda make up for the lack of React features here.

TLDR: as of right now, we cannot support renderToPipeableStream as React is lacking the primitives for us (or any other data fetching library) to do so. I'm really sorry.

kcoet commented 11 months ago

I was stuck in the same problem for several days.

I hope the Apollo team can provide examples of suggested SSRs now that as with libraries in general, they have an examples directory.

@phryneas Do you have any suggestions what is the best current method for building SSR with Apollo? is it still renderToStaticMarkup like before?

phryneas commented 11 months ago

@kcoet as I wrote above, renderToPipeableStream is literally impossible to use with any form of server-side data fetching (not just Apollo Client, also libraries like React Query or RTK Query - as I said above, React 18 doesn't contain the necessary primitives), you'd just do SSR as you did before it existed. No change in recommendations.

It sucks, but we cannot change it. I have a working prototype of hooks that I added myself to the React core to make this work, but of course we won't ask you to patch your react package.

As it stands, just stay clear of renderToPipeableStream. I'm sorry :/

phryneas commented 5 months ago

Small update on this - this is still difficult to set up, but not impossible. I'm currently in the process of writing docs on this, you can follow the PR in #11807 - I'd be happy for any feedback :)

alvaro-cuesta commented 5 months ago

For anyone interested, I hacked around this by only sending the renderToPipeableStream response on onAllReady.

So basically... by not streaming, but works for my use case since I'm just migrating from React 17 + Loadable Components and I needed Reac.lazy SSR support, which only renderToPipeableStream supports... and as a side effect the cache for suspended queries is fully populated on hydration, so there are no additional queries for me.

Sharing this in case someone is in a migration like me and got scared by this thread... there's hope!

Can't wait to have proper streaming implemented.