apollographql / apollo-client-nextjs

Apollo Client support for the Next.js App Router
https://www.npmjs.com/package/@apollo/experimental-nextjs-app-support
MIT License
459 stars 36 forks source link

Prefetching Data within RSCs #29

Open cjonesdoordash opened 1 year ago

cjonesdoordash commented 1 year ago

Hey, I've been taking a peek at how Apollo should be working with Next 13's app directory and found this comment: https://github.com/apollographql/apollo-client-nextjs/pull/9#issuecomment-1544005482 chatting a bit more around prefetching within server components and pushing that to client components. In the spirit of allowing for easier migration paths from the pages directory -> app directory we'd really like to use prefetching initially both at the layout level and page level so that we can more easily shift things over.

Has there been any movement around supporting page level prefetching and population to client components? It seems like layout level I should be able to forward the data over as props to the client component and then initialize my cache with it.

Thanks for all the awesome work supporting Next 13!

phryneas commented 1 year ago

It's on the list, but right now we had a lot of stuff on our hands, getting the Apollo Client 3.8 beta out of the door, and I'll be on conferences for the next week. Once I'm back, this is one of my first TODOs.

That said, I am not sure if this is the right tool for your (it rarely is for most use cases).

Can you explain a little more about "page level prefetching" vs "layout level prefetching" that you mean here? Conceptually, there should not be a lot of difference between those - and generally, there is no inherent need to do that kind of prefetching in RSC at all. You could always just make your page (or layout) a Client Component and use useSuspenseQuery in there - that will prefetch data on first page load during SSR and move it over to the client before rehydrating.

cjonesdoordash commented 1 year ago

So in general prefetching in the way that I described it is really only useful when it comes to helping migrate from the Next pages directory to the app directory since prefetching at both the layout and page level would give you a similar waterfall model. Definitely not something I would view as being needed if you're doing the true "correct" form of data fetching.

One other piece to call-out those is that the new Next.js Metadata API doesn't really work with this flow since your data fetching is done outside of RSC's; any thoughts on this? Basically if we'd like to support metadata from gql calls we'd need to make the call twice (once via useSuspenseQuery and the 2nd time via RSC to populate data to client-components and update metadata based on data) since the caches between these two are not shared.

I was able to get everything except metadata working with useSuspenseQuery to ensure that things are fetched on the server then moved over to the page before hydrating. However for a true layout level fetching experience (ie you have common data used between pages that you only want to initially fetch once) it would most likely be better to utilize the new useBackgroundQuery and useReadQuery. Based on this issue it looks like those are in progress now though.

cjonesdoordash commented 1 year ago

@phryneas any thoughts on the above?

phryneas commented 1 year ago

Yeah, sorry for the late reply, I'm just settling in after getting back :)

So in general prefetching in the way that I described it is really only useful when it comes to helping migrate from the Next pages directory to the app directory since prefetching at both the layout and page level would give you a similar waterfall model. Definitely not something I would view as being needed if you're doing the true "correct" form of data fetching.

Gotcha. I'm not sure if we can do a lot for layout <-> page though - that's pretty much a nextjs design decision.

One other piece to call-out those is that the new Next.js Metadata API doesn't really work with this flow since your data fetching is done outside of RSC's; any thoughts on this? Basically if we'd like to support metadata from gql calls we'd need to make the call twice (once via useSuspenseQuery and the 2nd time via RSC to populate data to client-components and update metadata based on data) since the caches between these two are not shared.

I have to be honest, I haven't worked with that yet, but I've given it a try just now.

From what I see, having a generateMetadata function and calling getClient() in there will return you the same ApolloClient instance as calling getClient() in a React Server Component in the same page, so duplicate queries you make there will be deduplicated between those two.

I was able to get everything except metadata working with useSuspenseQuery to ensure that things are fetched on the server then moved over to the page before hydrating. However for a true layout level fetching experience (ie you have common data used between pages that you only want to initially fetch once) it would most likely be better to utilize the new useBackgroundQuery and useReadQuery. Based on this https://github.com/apollographql/apollo-client/issues/10875 it looks like those are in progress now though.

Yup, we are on that :)

cjonesdoordash commented 1 year ago

So you can definitely get deduplicated queries for generateMetadata and queries within react server components.

However if you're using useSuspenseQuery within a client component that then runs on the server there's no way to have that be deduplicated which really effectively prevents you from using useSuspenseQuery for prefetching purposes if you also need to provide some of the prefetched data for metadata as well since you'd need to effectively duplicate the code + network call between a useSuspenseQuery within a client component and a getClient() within a react server component.

phryneas commented 1 year ago

That can hardly be prevented - Nextjs runs the RSC run and the SSR run in two completely different processes that cannot communicate with each other. It might be possible to use the Nextjs request cache for this (I do not know how they have set that up), but I do not have very high hopes.

cjonesdoordash commented 1 year ago

Yea I definitely agree that the two weren't built to work together in general. Was mainly just looking to call out that utilizing useSuspenseQuery might not be the best option in general when it comes to prefetching due to this limitation (at least if you need to update metadata with your prefetched data)

With that being said I'm looking forward 3.8 Beta and RSC based prefetching to bridge that gap 😄

phryneas commented 1 year ago

To be honest, I wouldn't be overly concerned though - not every single request to your GraphQL server is a bad thing, and if those requests happen on your app server, they are probably a lot closer to your "source of truth" and much faster than if a client would make them.