-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
docs(ssr): add experimental app directory guide #4568
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9fd54ef:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. I would like to structure it a bit differently:
let's keep everything as it is for the nextJs (12) docs and start a new section at the bottom of this page called Experimental app directory in Next 13
or something. There, we can put the initialData
and Hydrate
approaches, with another notice that this is experimental and best practices might change as we figure things out :)
docs/guides/ssr.md
Outdated
<QueryClientProvider client={queryClient}> | ||
<Hydrate state={dehydratedState}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be able to decouple QueryClientProvider
and Hydrate
. The Provider needs to go at the top, once. Nothing changes here really.
Hydrate
can appear multiple times in the app tree. It just takes data and puts it into the cache if it's newer, as opposed to initialData
, which will only write data once to the cache if there is no data yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decoupling it is a really good point. In the current implementation, you might end up prefetching data at the top level for a leaf component that is conditionally rendered. Effectively, you could end up prefetching data that isn't needed in the end. Fetching data closer to the component that needs it could also allow you to parameterize that fetch with arguments that wouldn't be available at the top level.
I'll play around with these ideas a bit and see what kind of new implementation I can get working. Thinking something along the lines of a higher-order component that does the fetching and wraps its argument in a Hydrate
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say keep it simple for now, the API is probably going to change in the next RQ major anyway. If you wrap QueryClientProvider
higher up in the tree, you can just do:
'use client';
import { Hydrate } from '@tanstack/react-query'
export default function HydrateOnClient(dehydratedState) {
return <Hydrate state={dehydratedState}>{children}</Hydrate>;
}
and use that directly in the Server Components. Don't think there is need for a HoC or something more complex here (at least yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://codesandbox.io/p/github/efilion/next13-react-query/decoupled-hydrate?file=%2Fapp%2Fpage.tsx
The HoC isn't strictly necessary, but it helps hide some of the complexity and keep things DRY.
To take advantage of the deduplication provided by QueryClient, the instance defined at the page level will need to be threaded down through to all components looking to prefetch data. That nullifies one of the advantages of the Hydration approach over the InitialData approach. It would be nice to have something akin to an IoC container that could provide a QueryClient as a request-scoped singleton, but I didn't see much in the way of options to accomplish that at the moment. InversifyJS and TSyringe don't fit the bill here. Likely a difficulty with the fact that being single-threaded, Node.js doesn't provide each request its own thread. Nest.js has a solution, so maybe Next.js could implement this itself down the line. Alternatively, can look at the Reader Monad pattern from functional programming to handle dependency injection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's a tricky one, clever solution!
I worry about putting that much complexity in the docs though, especially for an experimental approach, might fit better as an example to link to? There might be another way as well, not saying it's better but I thought I'd throw it out there.
So behind the scenes we have RSCs, client (server rendered) and client (browser rendered). For both of the client parts, it's important to have the same queryClient
throughout the whole rendering which is why we want to hoist it to the top, but for the RSCs, your initial approach is actually fine where you create a new queryClient
for each RSC (fine as in works, not as in pretty, DRY, fast etc). The reason for this is that since that data gets de/rehydrated anyway, we only need the queryClient
as a way to be able to call prefetchQuery
and get the right format back when we call dehydrate
. The fetches themselves will be deduped behind the scenes anyway, so even if two RSCs call the same endpoint, it at least wont double fetch.
So for RSCs specifically, it's fine to duplicate the queryClient
instead of passing it around. Ugly and not something we want long term, but might be easier and more concise to document, although possibly more confusing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but it helps hide some of the complexity and keep things DRY.
I guess another way of stating the above is that I'm not sure this is something we should aim to do at this point, I kind of like having the full extent of the experimental and raw nature of this fully visible, it becomes less of an "officially endorsed pattern" that way which is something that might bite us later (even with caveats, some devs will just copy-paste it). 😅
Maybe I worry too much though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for RSCs specifically, it's fine to duplicate the
queryClient
instead of passing it around. Ugly and not something we want long term, but might be easier and more concise to document, although possibly more confusing?
Fortunately, I finally stumbled across the section on Per-request Caching. That will allow me to create the request-scoped singleton I wanted without even having to bring in a 3rd party dependency. This way we only ever have to deal with a single value of dataUpdateAt
even if multiple components use the same query.
My only concern now is dehydrating only the queries relevant to the component being wrapped in <HydrateOnClient>
. Trying to avoid sending more data than necessary. Wondering if we should think about extending the dehydrate
API to match on queryKey/queryHash to improve on time complexity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but it helps hide some of the complexity and keep things DRY.
I guess another way of stating the above is that I'm not sure this is something we should aim to do at this point, I kind of like having the full extent of the experimental and raw nature of this fully visible, it becomes less of an "officially endorsed pattern" that way which is something that might bite us later (even with caveats, some devs will just copy-paste it). sweat_smile
Maybe I worry too much though.
I see what you mean. My HoC approach could be a better match for a blog post.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mainly looking to improve on line 12 inside hydratedContent.tsx
. Not setting anything at all here might be the best approach for the moment. Might need to make a change to dehydrate
to pick and choose which queries to dehydrate based on queryKey. Open to suggestions.
Agreed. My instinct was to have them together because they share things in common, and I didn't want to be too repetitive, but I'm not writing an essay here and the goal isn't to compare and contrast. When people read this, they'll just choose to read one section, so being repetitive isn't as bad. Better not to disrupt the flow of documentation that people are already used to or to introduce experimental stuff before fully addressing the more standard approach. |
docs/guides/ssr.md
Outdated
|
||
export default async function Providers({children}) { | ||
const queryClient = new QueryClient() | ||
await queryClient.prefetchQuery(['posts'], getPosts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems really backwards that you gotta await queryClient.prefetchQuery(['posts'], getPosts)
in order for the state to be hydrated...
I would expect some magic to hydrate anything that has been called with useQuery(..., { suspense: true })
automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a missing API in React for this at the moment? No way of pushing state from a server-render React-component to a client-side one without this stuff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server-side suspense is a whole other wrinkle I hadn't considered. Dropping reactwg/react-18#37 here for reference because I'll likely want to circle back on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this could be handled better by TanStack/query, but for now, it is what it is :). This is exactly why I'd like the whole section to be marked as experimental
- because the recommendations might change.
also looping in @Ephem :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, the recommendation should always be to prefetch things as early as possible, both to get a headstart and to avoid waterfalls where you suspend in A to be able to render B which then suspend again etc. We should definitely investigate supporting the other case as well though (if it's even going to be possible), but that's for the future.
One caveat is that currently you have to await the data in the RSC, which blocks streaming what you can early. What we really want is to start prefetching in the RSC but not await it, instead suspending in the component that needs this data. That will require both that React implements de/hydration of Promises and a bunch of changes to RQ though, so I think this PR (with the proposed changes elsewhere) documents the best current experimental approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and like we talked about on Discord @KATT, I think it's interesting here to think about how to support "normal" streaming rendering without RSCs as well. A lot of frameworks might implement streaming rendering without RSCs, so that's also something we want to support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, for now we should definitely document that useQuery(..., { suspense: true })
wont work on the server unless you have already prefetched that data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you've already prefetched your data, is there any utility to set a client-side suspense boundary? If you don't prefetch, you can set suspense boundaries inside a client-side component and use useQuery(..., { suspense: true })
. Data will start fetching after the page loads. If you're prefetching, you can set a suspense boundary in a server-side component, and there's no need for { suspense: true }
inside useQuery
. Data will start fetching as soon as possible, the server will send a response without waiting for it, and then stream the rendered component to be integrated into the page once the fetching completes.
docs/guides/ssr.md
Outdated
@@ -111,6 +180,95 @@ function Posts() { | |||
|
|||
As demonstrated, it's fine to prefetch some queries and let others fetch on the queryClient. This means you can control what content server renders or not by adding or removing `prefetchQuery` for a specific query. | |||
|
|||
#### With experimental `app` directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd add a big :::caution
under here that this is experimental and is subject to change awaiting {{some link to a React-issue or RFC}}
docs/guides/ssr.md
Outdated
|
||
Prefetch your data inside a Server Component and provide your previously defined Client Component with the dehydrated state of the server-side `QueryClient`. | ||
|
||
- Create a new QueryClient instance **for each page request. This ensures that data is not shared between users and requests.** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rephrase this somehow
- Create a new QueryClient instance **for each page request. This ensures that data is not shared between users and requests.** | |
- Create a new QueryClient instance **within the body of the React component**. This ensures that data is not shared between users and requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work on this so far! In a longer time perspective I think we have a lot to figure out about how to document and teach this for different cases, frameworks etc, but don't think there is much use in doing that before we figure out what the "next generation" of hydration etc is going to look like, this looks like a good stopgap for now for people that want to experiment. 😄
docs/guides/ssr.md
Outdated
React Query supports both of these forms of pre-rendering regardless of what platform you may be using | ||
|
||
### Using `initialData` | ||
|
||
The setup is minimal and this can be a quick solution for some cases, but there are a **few tradeoffs to consider** when compared to the full approach: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this section! It's been sorely missing, glad you included it, and well written. 😄
docs/guides/ssr.md
Outdated
<QueryClientProvider client={queryClient}> | ||
<Hydrate state={dehydratedState}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say keep it simple for now, the API is probably going to change in the next RQ major anyway. If you wrap QueryClientProvider
higher up in the tree, you can just do:
'use client';
import { Hydrate } from '@tanstack/react-query'
export default function HydrateOnClient(dehydratedState) {
return <Hydrate state={dehydratedState}>{children}</Hydrate>;
}
and use that directly in the Server Components. Don't think there is need for a HoC or something more complex here (at least yet).
docs/guides/ssr.md
Outdated
|
||
export default async function Providers({children}) { | ||
const queryClient = new QueryClient() | ||
await queryClient.prefetchQuery(['posts'], getPosts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, the recommendation should always be to prefetch things as early as possible, both to get a headstart and to avoid waterfalls where you suspend in A to be able to render B which then suspend again etc. We should definitely investigate supporting the other case as well though (if it's even going to be possible), but that's for the future.
One caveat is that currently you have to await the data in the RSC, which blocks streaming what you can early. What we really want is to start prefetching in the RSC but not await it, instead suspending in the component that needs this data. That will require both that React implements de/hydration of Promises and a bunch of changes to RQ though, so I think this PR (with the proposed changes elsewhere) documents the best current experimental approach.
Codecov ReportBase: 96.36% // Head: 91.25% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #4568 +/- ##
==========================================
- Coverage 96.36% 91.25% -5.11%
==========================================
Files 45 110 +65
Lines 2281 4116 +1835
Branches 640 1057 +417
==========================================
+ Hits 2198 3756 +1558
- Misses 80 339 +259
- Partials 3 21 +18 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Line 218: Wondering if it would be a good idea to mention that eventually 'use client' will be added to |
Line 248: Looking only to dehydrate the queries that were prefetched in the Server Component. Could export |
I haven't had time yet to look at the latest changes here (soon!), but I thought I'd note that I've written a braindump discussion "React Streaming SSR/Server Components" that relates to this PR. |
Thanks for writing this up! Tons of really useful background/context. Will take a bit for me to digest all of it and get up to speed. Already, I'm thinking I should just drop the query cache filtering on line 248, and wait to see what new API will be settled on. Maybe I make a note that the current approach serializes and sends more duplicate data to the client than necessary, but that this will be addressed in a later update. Also, I think I still need to think more thoroughly about what will happen with my current approach if someone opts in to concurrent features... especially Suspense. I can see that the query cache could look different between the time a component renders on the server and the time it hydrates on the client. Two components share a query that wasn't prefetched on the server, one component hydrates and fetches the query before the second can hydrate. Or maybe a component has access to data on the server because the query was already prefetched by another component, but then on the client it hydrates before the component with the |
Yeah, this can be a tradeoff in some cases, but I think compression (gzip/brotli) might take care of a lot of that since the data would be identical, and I'm not sure if React itself might even try to do something smart here when props are identical? Still something to keep an eye on though.
When we fix After hydration has happened, the component would rerender with the fresh data. This does mean that unhydrated components will be stale while others might have already updated though, but that's unavoidable with incremental hydration. |
docs/guides/ssr.md
Outdated
export default getQueryClient | ||
``` | ||
|
||
Re-export the `<Hydrate>` component as a Client Component. This allows `<Hydrate>` to execute on the client and make use of client-only features. **TODO: Plans to add 'use client' to react-query modules?** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add: This is a temporary workaround until 'use client'
is eventually added directly into the react-query
modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we have plans to do this. @Ephem this is something we could even do for v4, right?
docs/guides/ssr.md
Outdated
await queryClient.prefetchQuery(['posts'], getPosts) | ||
const dehydratedState = dehydrate(queryClient, { | ||
dehydrateMutations: false, | ||
shouldDehydrateQuery: query => true // TODO: matchQuery or extend dehydrate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop this for now and simply have const dehydratedState = dehydrate(queryClient)
Add an acknowledgement that more duplicate data than strictly needed is being serialized and sent to the client, and that this issue is being looked at for a future update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, we should probably just use the default settings here. There likely are no mutations, so setting dehydrateMutations
to false
doesn't accomplish much
@HamAndRock That's weird. Some questions:
|
Hey, thank you for answer, the issue was in the import of dehydrate like you mentioned I used autocomplete to import it and it picked the react-query version instead of query-core. I am sorry, this is my fault for wasting your time. I suggest maybe adding some kind of warning to the docs about these two "identical" imports, so it's easier to find the issue faster. Thank you again and sorry for me being not able to copy code :D |
Not wasting our time at all. Really valuable feedback into improving DX of the docs. I think adding that kind of warning is a good idea. As you mentioned, most IDEs will autocomplete that line to use the wrong module, and it's very easy for our eyes to glaze over such a small detail. |
I think we have a PR that adds a lot of utility as is, so I would suggest merging it after todos 1-4 are completed. Any refinements based on todo 5 could be addressed in a later PR. |
We have a PR that adds
I think the PR is good to go, but if you want to give it a review, please do. Would this solve the problem?
👍 |
I think we can land #4738 first, I think it needs some changes but should be pretty quick? So if you want to modify this to remove the |
I took the liberty to push some changes to this, I hope that was okay.
I think with those changes this should be ready to merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow this is really good 👍
Suspense section is very well put. Thanks for taking care of that! Thrilled to see this PR get shipped :) |
There's an issue to track this here, which includes a work around: #4933 |
Adding Next.js 13 app directory docs discussed in #4558.
The initialData approach was first published by @lukemorales. This approach appears to be subject to the same caveats as with using initialData in the standard
pages
directory. Having to threadinitialData
through to a component deep down in a component tree is aggravated by now having to fetch data in an ancestor Server Component.Given those drawbacks, I've also documented an approach that rehydrates the Query Cache. This approach was largely informed by Using Other Frameworks or Custom SSR Frameworks. There are two notable deviations from that guide:
queryClient.clear()
. As per @TkDodo, this is no longer necessary since v4.