-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
WIP: Improve support for server side rendering #570
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
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) |
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.
Would love to see codesandbox (using Next.js) for these
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'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!
|
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 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? |
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 React Query already has a great "delivery mechanism", the 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? |
|
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 |
| if (!isServer) { | ||
| cache.queries[queryHash].scheduleStaleTimeout() | ||
| // Schedule for garbage collection in case | ||
| // nothing subscribes to this query | ||
| cache.queries[queryHash].scheduleGarbageCollection() | ||
| } |
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.
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?
|
What if all of the server side stuff was in its own import? react-query/ssr?
…On Jun 11, 2020, 1:35 AM -0600, Fredrik Höglund ***@***.***>, wrote:
@Ephem commented on this pull request.
In src/queryCache.js:
> + if (!isServer) {
+ cache.queries[queryHash].scheduleStaleTimeout()
+ // Schedule for garbage collection in case
+ // nothing subscribes to this query
+ cache.queries[queryHash].scheduleGarbageCollection()
+ }
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?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Again a great question, I think so and it got me thinking about a slightly alternative approach. This PR in essence does two things:
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
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? |
|
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. |
|
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 |
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:
|
|
Sounds like a good plan :) |
|
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. |
|
Surprise! The |
|
Great! I created a new PR with the breaking change I mentioned. It still uses the 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. 😄 |
Closes #461
This PR adds:
makeServerQueryCache()- Create aqueryCachethat caches queries in a server environmentqueryCache.dehydrate()- A method that outputs a serializable version of the cacheinitialQueries-option tomakeQueryCachethat creates a pre-warmed cache from dehydrated queriesTogether, 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
initialQueriesare 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
makeServerQueryCachecaches data on the server. An alternative approach that would technically be a breaking change is to enable query caches returned by the regularmakeQueryCacheto 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.prefetchQuerieson that cache, using the returned result, but not expecting it to end up in the cache.Missing/Future/Follow up?
initialQueries?queryCache.refetchQueriesafter hydration to trigger a refetch for up to date data