Skip to content

Conversation

@Ephem
Copy link
Collaborator

@Ephem Ephem commented Jun 10, 2020

Closes #461

This PR should be complete, but I marked it as WIP since I haven't had time to verify everything a 100% myself and will soon be on a weekisch long vacation. Since it's a largeish change in scope and I expect it will take some time to review and think about I wanted to get it up as soon as possible.

This PR adds:

  • makeServerQueryCache() - Create a queryCache that caches queries in a server environment
  • queryCache.dehydrate() - A method that outputs a serializable version of the cache
  • A initialQueries-option to makeQueryCache that creates a pre-warmed cache from dehydrated queries
  • Tests, docs and an example for the above

Together, these new APIs form the missing pieces for proper SSR-support. Docs and the example is a good place to start reviewing.

Caching on the server isn't a huge change conceptually now that caches can be provided via ReactQueryCacheProvider, but I think the de/rehydration parts require some close scrutiny and thinking about.

Caveats

  • initialQueries are scheduled for stale timeout from the time of client cache creation + staleTime, not from the time they were fetched on the server. Using the server time is tricky since server and client time might be out of sync.

Breaking vs non-breaking and an alternative approach

This PR is not a breaking change since only caches created by the new makeServerQueryCache caches data on the server. An alternative approach that would technically be a breaking change is to enable query caches returned by the regular makeQueryCache to also cache data on the server (but still not the global one).

The reason this would be breaking is that it is today technically possible to create such a cache on the server and do queryCache.prefetchQueries on that cache, using the returned result, but not expecting it to end up in the cache.

Missing/Future/Follow up?

  • Types
  • Improved devTools-support?
    • Seeing which queries were rehydrated via initialQueries?
    • I haven't used the devtools myself, so there might be opportunities I'm missing
  • More examples (Next, Gatsby?)
  • Improve SSR-docs with different scenarios/cases?
    • For example, when caching the server output, you can call queryCache.refetchQueries after hydration to trigger a refetch for up to date data
  • Provide a way to configure automatically refetching data after hydration?
  • Add server Suspense support (currently not supported by the official renderer, only with experimental things like react-ssr-prepass or react-lightyear)

Ephem added 4 commits June 10, 2020 17:08
Add makeServerQueryCache as a way to create a queryCache that caches data on the server.
Add queryCache.dehydrate as a way to dehydrate the cache into a serializeable format.
Add initialQueries as an option to makeQueryCache as a way to rehydrate/create a warm cache.

Closes TanStack#461
- Star Wars (with devtools) - [CodeSandbox](https://codesandbox.io/s/github/tannerlinsley/react-query/tree/master/examples/star-wars) - [Source](./examples/star-wars)
- Rick And Morty (with devtools) - [CodeSandbox](https://codesandbox.io/s/github/tannerlinsley/react-query/tree/master/examples/rick-morty) - [Source](./examples/rick-morty)
- Prefetching - [Source](./examples/prefetching)
- Server Side Rendering - [Source](./examples/server-side-rendering)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would love to see codesandbox (using Next.js) for these

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been wanting to try out Codesandbox with a server for a while so that sounds good, I'll see what I can do.

I added the existing Prefetching example here just because I saw it was missing from the Readme. Adding Codesandbox for that here would be scope creep, want me to remove it again until someone can add a sandbox?

I wanted to include a "low level" SSR-example in this PR to demo the nitty gritty, I think a Next-example is the logical follow-up. I kind of think that example should also be included in this PR to demo that it's compatible, I'll go ahead and add that after the vacation!

@tannerlinsley
Copy link
Member

Something that is on my mind as I'm reading through this: It doesn't feel like any of this is necessary when using something like Next.js right? I understand this pattern really well, as this is exactly what I did in React Static when I build that, but this is assuming that React Query is taking on more responsibility of how the data is captured and transferred to the client for rehydration. Whereas in Next.js case, all you have to do is go prefetch the data, feed it to getServerSideProps or getStaticProps and then pass it as initialData to your query. Do you see where I'm coming from?

With that said, it's starting to feel like all of this logic, while being totally necessary for SSR to work properly is less of React Query's concern and more of an SSR framework/setup concern. Another red flag here is that Apollo Client got into this territory too and ended up with a very intense SSR model that blurred the lines of responsibility IMO between what is library and platform.

Thoughts?

@Ephem
Copy link
Collaborator Author

Ephem commented Jun 10, 2020

Thoughts?

That's a great question! I read it as "What do we gain by adding this complexity?", so I'll try to address that.

I think the main thing is that in Next for example, you only have access to the data you prefetched at the top route-component, while React Query is designed to be able to fetch data anywhere. Using React Query with something like Next currently mean you have to somehow pass that prefetched data down (via props or context) to be used as initialData in every place you useQuery. This essentially forces you to implement a parallel data-cache in user-space for use server side and then the point of using React Query in the first place is heavily diluted.

React Query already has a great "delivery mechanism", the queryCache so this PR aims to make that available in all environments. If you already have that, dehydrating that directly vs manually keeping track of all the data that was fetched and manually adding that the data to the cache at a top level (for example via queryCache.setQueryData) seems to make sense.

It is very possible that the de/rehydration parts could be solved in different/simpler ways, I'm very open to suggestions there. I strongly think having a universal cache that you can read data from the same way on the server and the client is essential for any kind of ergonomic use of React Query with SSR as soon as you start fetching data any place other than root route components though.

This definitely is a scope change for the library so the question is very valid and important and I would love to go into more depth, but I'll stop there for now to hear your thoughts on the above reasoning?

@Ephem
Copy link
Collaborator Author

Ephem commented Jun 10, 2020

Oh and btw, I agree with the blurred lines between framework and data fetching. I think a lot of the complexity Apollo took on for example stemmed from them trying to make things ergonomic that aren't currently supported by React/frameworks. Specifically solving the "fetch data inside of render even on the server"-problem.

I don't think React Query should try to solve either that or how you get a hold of the queries you are prefetching or where you do it. In other words, exactly where and how the serverQueryCache.prefetchQuery happens should definitely be a framework/router/app-architecture concern.

Comment on lines +603 to +608
if (!isServer) {
cache.queries[queryHash].scheduleStaleTimeout()
// Schedule for garbage collection in case
// nothing subscribes to this query
cache.queries[queryHash].scheduleGarbageCollection()
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon reflection, this block is a mistake. Side effects when creating things isn't very good practice and for Next for example we want makeQueryCache to be able to run inside of render.

One solution would be to move this into an init-function and let ReactQueryCacheProvider check if the cache has been/needs to be initialized, in that case that happens in an effect. A custom cache should always be paired with a cache provider, so this should provide an equally simple API for users, but be a safe way to do it.

Another option would be to add the prop initialQueries directly to the cache provider to avoid having to create a store at all on the client, which could be an ever nicer API?

@tannerlinsley
Copy link
Member

tannerlinsley commented Jun 11, 2020 via email

@Ephem
Copy link
Collaborator Author

Ephem commented Jun 11, 2020

What if all of the server side stuff was in its own import? react-query/ssr?

Again a great question, I think so and it got me thinking about a slightly alternative approach. This PR in essence does two things:

  • Adds an extra "serverEnabled" conditional for "should I save this query to cache or not"

After doing some thinking, maybe the concept of "frozen" vs "unfrozen" caches makes more sense? All caches start as frozen on the server by default, but you can .unfreeze() them (and even .freeze() them again?). This part needs to stay in the queryCache to avoid a breaking change, but is very minimal, non-intrusive and doesn't really add complexity.

  • The big one: Adds de/rehydrate functionality

I think this one could probably use a functional approach instead (naming could need work):

import { makeQueryCache } from 'react-query'
import { dehydrateCache, rehydrateCache } from 'react-query/some-scope' 

// Next line would look a little different without a breaking change:
const serverCache = makeQueryCache()
const dehydratedQueries = dehydrateCache(serverCache)

const clientCache = makeQueryCache()
rehydrateCache(clientCache, dehydratedQueries)

One might start questioning why this should even be part of React Query in that case, but this would rely on internals, so it would be unsafe to implement in user-space.

A version closer to what you are suggesting is:

import { makeQueryCache } from 'react-query/ssr'

const serverCache = makeQueryCache()
const dehydratedQueries = serverCache.dehydrate()

// Note that this needs to use the same import since rehydration is also included there:
const clientCache = makeQueryCache({ initialQueries: dehydratedQueries })

Both have merits and I like both better than the existing PR both to avoid increasing payload for the default use case unnecessarily and to separate concerns. What do you think?

@tannerlinsley
Copy link
Member

The functional approach is really cool actually. Imagine if someone wanted to dehydrate a client side cache to store in local storage? I totally agree on the bundle savings by keeping it separate. Can’t rely on tree shaking in every environment so this would draw a clear boundary between the core and such a utility.

@tannerlinsley
Copy link
Member

Also, with the future in mind, we can really focus on improving the size and integration even more when we choose to do 3.0

@Ephem
Copy link
Collaborator Author

Ephem commented Jun 11, 2020

The functional approach is really cool actually. Imagine if someone wanted to dehydrate a client side cache to store in local storage?

Yeah, this is another use case I have been thinking about, this could help solve scroll restoration on reload among other things. :)

If you approve, I think I'll go ahead and split this PR when I have time:

  • Add frozen/unfrozen concept
  • Add separate entry with de/rehydration
  • Examples

@tannerlinsley
Copy link
Member

Sounds like a good plan :)

@tannerlinsley
Copy link
Member

Also, if there are things you think would be better as a breaking change, I would be fine talking about them. I would rather do things right than avoid backwards compatibility at the cost of strangeness.

@tannerlinsley
Copy link
Member

Surprise! The next branch is ready for testing and I think we should push this change to the next branch and also make any breaking changes to it to make this all work even better.

@Ephem
Copy link
Collaborator Author

Ephem commented Jun 14, 2020

Great! I created a new PR with the breaking change I mentioned. It still uses the frozen concept internally, but that doesn't need to be exposed to users now that we are targeting a major. 🎉

I'll go ahead and close this PR now that it's being broken apart into pieces instead. I added a summary of the next steps to the issue. 😄

@Ephem Ephem closed this Jun 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support per-request cache with SSR

2 participants