query icon indicating copy to clipboard operation
query copied to clipboard

docs(ssr): add experimental app directory guide

Open efilion opened this issue 3 years ago • 8 comments

Adding Next.js 13 app directory docs discussed in https://github.com/TanStack/query/discussions/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 thread initialData 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:

  • Not calling queryClient.clear(). As per @TkDodo, this is no longer necessary since v4.
  • Not populating window.REACT_QUERY_STATE with dehydratedState. Next.js is handling hydration without any need for this.

efilion avatar Nov 28 '22 23:11 efilion

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 9fd54ef01ee88b87fb7eb328650e653fdcadf8df:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

codesandbox-ci[bot] avatar Nov 28 '22 23:11 codesandbox-ci[bot]

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 :)

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.

efilion avatar Nov 29 '22 22:11 efilion

Codecov Report

Base: 96.36% // Head: 91.25% // Decreases project coverage by -5.11% :warning:

Coverage data is based on head (08d6c84) compared to base (eab6e2c). Patch has no changes to coverable lines.

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     
Impacted Files Coverage Δ
src/core/notifyManager.ts
src/core/subscribable.ts
src/core/query.ts
src/core/utils.ts
src/devtools/useLocalStorage.ts
src/react/useIsMutating.ts
src/devtools/devtools.tsx
src/devtools/theme.tsx
src/core/queryCache.ts
src/devtools/utils.ts
... and 145 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Dec 10 '22 07:12 codecov-commenter

Line 218: Wondering if it would be a good idea to mention that eventually 'use client' will be added to react-query modules or that this is still being evaluated.

efilion avatar Dec 12 '22 05:12 efilion

Line 248: Looking only to dehydrate the queries that were prefetched in the Server Component. Could export matchQuery from query-core, but will need to make the types match, and time complexity would be less than ideal. Alternatively, could expand the dehydrate API to accept a list of queryKeys that can be looked up by queryHash.

efilion avatar Dec 12 '22 05:12 efilion

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.

Ephem avatar Dec 12 '22 13:12 Ephem

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 <Hydrate> wrapper can rehydrate the query cache with the prefetched query. Even refetching of a query that was initially prefetched on the server could be problematic.

efilion avatar Dec 12 '22 23:12 efilion

sends more duplicate data to the client than necessary

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.

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

When we fix useSyncExternalStore should have a correct/frozen getServerSnapshot (minor), that will take care of this. That makes it so during hydration, whenever that happens, all RQ hooks etc reads the original data that was passed down from the server, even if it has later changed. :smiley:

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.

Ephem avatar Dec 13 '22 11:12 Ephem

Hey, trying to use the Hydrate approach but getting this error, when adding the dehydrate() step to server component, runninng "@tanstack/react-query": "^4.22.0".

image

Is this supposed to happen since this is still a draft ?

HamAndRock avatar Jan 16 '23 14:01 HamAndRock

@HamAndRock That's weird. Some questions:

  • Does this happen even if you only call dehydrate, but not the Hydrate component?
  • If yes, where are you importing dehydrate from? It lives in the @tanstack/query-core package, so just importing and using that should absolutely not give you that error (but it's also re-exported in @tanstack/react-query which is why I'm asking)
  • If it only happens with the Hydrate component, did you remember to wrap it in a HydrateOnClient component with 'use client' on top?

Ephem avatar Jan 16 '23 15:01 Ephem

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

HamAndRock avatar Jan 16 '23 15:01 HamAndRock

@Ephem @efilion what would it take for us to get this from a draft PR to something that we can merge?

TkDodo avatar Jan 16 '23 15:01 TkDodo

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.

efilion avatar Jan 16 '23 22:01 efilion

@Ephem @efilion what would it take for us to get this from a draft PR to something that we can merge?

    • [x] Drop dehydrate options
  • As @Ephem mentioned, compression mitigates some of the problems with sending duplicate data to the client. I don't think this is a detail that needs to be added to the docs, but this issue may be a worthwhile consideration for future versions of the library.
  • I will make this change shortly.
    • [x] Make layout.jsx more consistent with create-next-app template
  • I will make this change shortly.
    • [ ] Mention that v4 targets a fix for having to re-export <Hydrate> simply to add use client
  • Waiting for confirmation on this. Alternatively, we can opt not mention anything in the docs at the present time.
    • [x] Add a note on importing from react-query vs query-core
  • I will make this change shortly.
    • [ ] Further investigation/testing into the behaviour of Suspense and other concurrent features
  • I think this can be made optional for now by stressing in the Suspense section that support for such is still experimental. Maybe add a prompt to create an issue if unexpected behaviour is encountered.

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.

efilion avatar Jan 16 '23 22:01 efilion

Waiting for confirmation on this. Alternatively, we can opt not mention anything in the docs at the present time.

We have a PR that adds useClient everywhere where we use state, context or effects:

  • https://github.com/TanStack/query/pull/4738

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 this can be made optional for now by stressing in the Suspense section that support for such is still experimental.

👍

TkDodo avatar Jan 20 '23 10:01 TkDodo

I think we can land https://github.com/TanStack/query/pull/4738 first, I think it needs some changes but should be pretty quick?

So if you want to modify this to remove the HydrateOnClient-wrapper, feel free to! When the above PR has landed, we might want to note the exact version in these docs? I'm totally fine with merging these docs first and changing that later too though. :smile:

Ephem avatar Jan 20 '23 13:01 Ephem

I took the liberty to push some changes to this, I hope that was okay.

  • Removed HydrateOnClient in favour of Hydrate (that PR is ready to be merged now`
  • Rewrote the Suspense-section
  • Some minor tweaks, including removing the note about importing directly from the core package since I'm not sure that still applies after we merge the 'use client' PR
  • Prettier
  • main had new docs on integrating with Remix, so I moved this whole section down to below those (since this is beta stuff) and added a note to the Next-section

I think with those changes this should be ready to merge ~~after https://github.com/TanStack/query/pull/4738~~ (update: now merged!), but I'd love it if someone could take a quick look. :smile: Great work on this @efilion!

Ephem avatar Jan 24 '23 09:01 Ephem

Suspense section is very well put. Thanks for taking care of that! Thrilled to see this PR get shipped :)

efilion avatar Jan 24 '23 19:01 efilion

image Hi. I'm trying to use Hydrate on server components but getting this error. Used HydrateOnClient component under QueryClient. Wrapped HydrateOnClient with another component and used at app/layout.tsx. I'm trying to achieve auth mechanism for routes. Can you guide me please? image

hakankaan avatar Mar 09 '23 16:03 hakankaan

image

Hi. I'm trying to use Hydrate on server components but getting this error. Used HydrateOnClient component under QueryClient. Wrapped HydrateOnClient with another component and used at app/layout.tsx. I'm trying to achieve auth mechanism for routes. Can you guide me please? image

I encountered the same problem, did you solve it?

StringKe avatar Mar 15 '23 01:03 StringKe

There's an issue to track this here, which includes a work around: https://github.com/TanStack/query/issues/4933

Ephem avatar Mar 15 '23 06:03 Ephem