diff --git a/.changeset/tidy-pugs-listen.md b/.changeset/tidy-pugs-listen.md new file mode 100644 index 00000000000..68d5b9cad21 --- /dev/null +++ b/.changeset/tidy-pugs-listen.md @@ -0,0 +1,5 @@ +--- +'@tanstack/preact-query': patch +--- + +Hydrate queries nobody is observing as soon as `HydrationBoundary` commits, so a `useQuery` that remounts under it no longer refetches data the dehydrated state already contains. diff --git a/packages/preact-query/src/HydrationBoundary.tsx b/packages/preact-query/src/HydrationBoundary.tsx index 1f860f9e385..ca7dc429317 100644 --- a/packages/preact-query/src/HydrationBoundary.tsx +++ b/packages/preact-query/src/HydrationBoundary.tsx @@ -7,8 +7,9 @@ import type { } from '@tanstack/query-core' import { Fragment } from 'preact' import type { ComponentChildren } from 'preact' -import { useEffect, useMemo, useRef } from 'preact/hooks' +import { useEffect, useLayoutEffect, useMemo, useRef } from 'preact/hooks' +import { useIsRestoring } from './IsRestoringProvider' import { useQueryClient } from './QueryClientProvider' export interface HydrationBoundaryProps { @@ -30,9 +31,10 @@ export const HydrationBoundary = ({ queryClient, }: HydrationBoundaryProps) => { const client = useQueryClient(queryClient) + const isRestoring = useIsRestoring() const optionsRef = useRef(options) - useEffect(() => { + useLayoutEffect(() => { optionsRef.current = options }) @@ -51,6 +53,9 @@ export const HydrationBoundary = ({ // If the transition is aborted, we will have hydrated any _new_ queries, but // we throw away the fresh data for any existing ones to avoid unexpectedly // updating the UI. + // + // Queries with no subscribers are the exception, they are hydrated as soon as + // the tree commits, see the layout effect below. const hydrationQueue: DehydratedState['queries'] | undefined = useMemo(() => { if (state) { if (typeof state !== 'object') { @@ -99,11 +104,57 @@ export const HydrationBoundary = ({ return undefined }, [client, state]) + // What the layout effect below leaves for the passive one, so that a query + // isn't hydrated, and its data deserialized, twice for the same commit. + const deferredRef = useRef(undefined) + + // Waiting for a passive effect is too late for a query with no subscribers. + // Children subscribe to the cache from their own passive effects and those + // run before the parent's, so a query that remounts under this boundary reads + // the old entry, finds it stale and refetches the very data we are holding. + // A query nobody subscribed to is also a query nobody gets notified about, so + // hydrating it as the tree commits doesn't update anything on the page, which + // is the only reason existing queries wait in the first place. + // + // While restoring, subscriptions are held back on purpose and that reasoning + // no longer applies, so everything waits for the passive effect. + useLayoutEffect(() => { + if (!hydrationQueue || isRestoring) { + deferredRef.current = hydrationQueue + return + } + + const queryCache = client.getQueryCache() + const unobserved: DehydratedState['queries'] = [] + const observed: DehydratedState['queries'] = [] + + for (const dehydratedQuery of hydrationQueue) { + const query = queryCache.get(dehydratedQuery.queryHash) + + if (!query || query.getObserversCount() === 0) { + unobserved.push(dehydratedQuery) + } else { + observed.push(dehydratedQuery) + } + } + + deferredRef.current = observed + + if (unobserved.length > 0) { + hydrate(client, { queries: unobserved }, optionsRef.current) + } + }, [client, hydrationQueue, isRestoring]) + + // Queries with subscribers keep waiting, so a render that ends up being + // thrown away, because something in it suspended, leaves the page the user is + // looking at alone. useEffect(() => { - if (hydrationQueue) { - hydrate(client, { queries: hydrationQueue }, optionsRef.current) + const deferred = deferredRef.current + + if (deferred && deferred.length > 0) { + hydrate(client, { queries: deferred }, optionsRef.current) } - }, [client, hydrationQueue]) + }, [client, hydrationQueue, isRestoring]) return {children} } diff --git a/packages/preact-query/src/__tests__/HydrationBoundary.test.tsx b/packages/preact-query/src/__tests__/HydrationBoundary.test.tsx index f0794fc7e88..bf4c99b3a8b 100644 --- a/packages/preact-query/src/__tests__/HydrationBoundary.test.tsx +++ b/packages/preact-query/src/__tests__/HydrationBoundary.test.tsx @@ -7,6 +7,7 @@ import { afterEach, beforeEach, describe, expect, vi, it } from 'vitest' import { HydrationBoundary, + IsRestoringProvider, QueryClient, QueryClientProvider, dehydrate, @@ -167,13 +168,15 @@ describe('Preact hydration', () => { queryClient.clear() }) - // When we hydrate in transitions that are later aborted, it could be - // confusing to both developers and users if we suddenly updated existing - // state on the screen (why did this update when it was not stale, nothing - // remounted, I didn't change tabs etc?). - // Any queries that does not exist in the cache yet can still be hydrated - // since they don't have any observers on the current page that would update. - it('should hydrate new but not existing queries if transition is aborted', async () => { + // Preact has no equivalent of a render that never happened. A tree that + // suspends is diffed and committed, layout effects and all, and only then + // gets swapped out for the fallback. What keeps the current page intact is + // that queries with observers wait for a passive effect, and those never + // run for a tree that suspended, see the test further down with a sidebar + // that survives the transition. Here the rerender introduces a new root, + // which remounts everything and unsubscribes the observer before the + // boundary even renders, so by then this is a query nobody is watching. + it('should hydrate an unobserved query even if the render that carried it suspends', async () => { const initialDehydratedState = JSON.parse(stringifiedState) const queryClient = new QueryClient() @@ -235,6 +238,11 @@ describe('Preact hydration', () => { ) expect(rendered.getByText('loading')).toBeInTheDocument() + // The tree committed before the fallback took over, so its data is in + // the cache even though none of it made it to the screen + expect(queryClient.getQueryData(stringKey)).toEqual([ + 'should not change', + ]) }) startTransition(() => { @@ -247,22 +255,16 @@ describe('Preact hydration', () => { , ) - // This query existed before the transition so it should stay the same - expect(rendered.getByText(stringKey[0]!)).toBeInTheDocument() - expect( - rendered.queryByText('should not change'), - ).not.toBeInTheDocument() + // Both pages render what the cache holds, the query that already + // existed included + expect(rendered.getByText('should not change')).toBeInTheDocument() + expect(rendered.queryByText(stringKey[0]!)).not.toBeInTheDocument() // New query data should be available immediately because it was // hydrated in the previous transition, even though the new dehydrated // state did not contain it expect(rendered.getByText(addedKey[0]!)).toBeInTheDocument() }) - await vi.advanceTimersByTimeAsync(20) - // It should stay the same even after effects have had a chance to run - expect(rendered.getByText(stringKey[0]!)).toBeInTheDocument() - expect(rendered.queryByText('should not change')).not.toBeInTheDocument() - queryClient.clear() }) @@ -311,6 +313,286 @@ describe('Preact hydration', () => { }) }) + it('should not refetch an inactive query when hydrated data is fresh', async () => { + const key = queryKey() + const queryClient = new QueryClient() + const queryFn = vi.fn(() => sleep(10).then(() => 'client')) + + function Page() { + const { data } = useQuery({ + queryKey: key, + queryFn, + staleTime: 1000, + }) + return
{data}
+ } + + // First visit fetches and caches the data + const rendered = render( + + + , + ) + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('client')).toBeInTheDocument() + + // Navigate away, the cached data goes stale while the page is unmounted + rendered.rerender( + +
+ , + ) + await vi.advanceTimersByTimeAsync(2000) + + // A loader fetches fresh data for the revisit and dehydrates it + const loaderClient = new QueryClient() + loaderClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'loader'), + }) + await vi.advanceTimersByTimeAsync(10) + const dehydratedState = dehydrate(loaderClient) + loaderClient.clear() + + queryFn.mockClear() + rendered.rerender( + + + + + , + ) + + // Hydration lands before the remounted useQuery subscribes, so it uses the + // fresh data instead of fetching it all over again + await vi.advanceTimersByTimeAsync(11) + expect(queryFn).toHaveBeenCalledTimes(0) + expect(rendered.getByText('loader')).toBeInTheDocument() + + queryClient.clear() + }) + + it('should not refetch a query that remounts in the same commit as the boundary', async () => { + const key = queryKey() + const queryClient = new QueryClient() + const queryFn = vi.fn(() => sleep(10).then(() => 'client')) + + function Page() { + const { data } = useQuery({ + queryKey: key, + queryFn, + staleTime: 1000, + }) + return
{data}
+ } + + const rendered = render( + + + , + ) + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('client')).toBeInTheDocument() + await vi.advanceTimersByTimeAsync(2000) + + const loaderClient = new QueryClient() + loaderClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'loader'), + }) + await vi.advanceTimersByTimeAsync(10) + const dehydratedState = dehydrate(loaderClient) + loaderClient.clear() + + queryFn.mockClear() + // Wrapping the page in the boundary remounts it, so the old observer goes + // away and a new one subscribes in one go. Preact tears the old tree down + // while it diffs, so by the time the boundary commits the query has no + // subscribers left + rendered.rerender( + + + + + , + ) + + await vi.advanceTimersByTimeAsync(11) + expect(queryFn).toHaveBeenCalledTimes(0) + expect(rendered.getByText('loader')).toBeInTheDocument() + + queryClient.clear() + }) + + it('should not hydrate a query that is on screen while a sibling suspends', async () => { + const key = queryKey() + const queryClient = new QueryClient() + + function Sidebar() { + const { data } = useQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'sidebar'), + staleTime: Infinity, + }) + return
{data}
+ } + + function Thrower(): never { + throw new Promise(() => { + // Never resolve + }) + } + + const rendered = render( + + + , + ) + await vi.advanceTimersByTimeAsync(11) + expect(rendered.getByText('sidebar')).toBeInTheDocument() + + const loaderClient = new QueryClient() + loaderClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'loader'), + }) + await vi.advanceTimersByTimeAsync(10) + const dehydratedState = dehydrate(loaderClient) + loaderClient.clear() + + // The route the app is navigating to suspends and never gets there, while + // the sidebar stays mounted and keeps rendering the query + rendered.rerender( + + + + + + + + , + ) + + expect(rendered.getByText('loading')).toBeInTheDocument() + await vi.advanceTimersByTimeAsync(100) + expect(rendered.getByText('sidebar')).toBeInTheDocument() + expect(queryClient.getQueryData(key)).toBe('sidebar') + + queryClient.clear() + }) + + it('should not hydrate an unsubscribed query while restoring', async () => { + const key = queryKey() + const queryClient = new QueryClient() + + queryClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'cached'), + }) + await vi.advanceTimersByTimeAsync(10) + + const loaderClient = new QueryClient() + loaderClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'loader'), + }) + await vi.advanceTimersByTimeAsync(10) + const dehydratedState = dehydrate(loaderClient) + loaderClient.clear() + + function Page() { + const { data } = useQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'client'), + }) + return
{data}
+ } + + function Thrower(): never { + throw new Promise(() => { + // Never resolve + }) + } + + const rendered = render( + + + + + , + ) + expect(rendered.getByText('cached')).toBeInTheDocument() + + // Nothing subscribes while restoring, so a cache entry that is on screen + // looks exactly like one nobody is using + rendered.rerender( + + + + + + + + + + , + ) + + expect(rendered.getByText('loading')).toBeInTheDocument() + await vi.advanceTimersByTimeAsync(100) + expect(rendered.getByText('cached')).toBeInTheDocument() + expect(queryClient.getQueryData(key)).toBe('cached') + + queryClient.clear() + }) + + it('should not deserialize the same query twice', async () => { + const key = queryKey() + const deserializeData = vi.fn((data: any) => data) + const queryClient = new QueryClient({ + defaultOptions: { hydrate: { deserializeData } }, + }) + + queryClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'cached'), + }) + await vi.advanceTimersByTimeAsync(10) + + const loaderClient = new QueryClient() + loaderClient.prefetchQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'loader'), + }) + await vi.advanceTimersByTimeAsync(10) + const dehydratedState = dehydrate(loaderClient) + loaderClient.clear() + + function Page() { + const { data } = useQuery({ + queryKey: key, + queryFn: () => sleep(10).then(() => 'client'), + staleTime: 1000, + }) + return
{data}
+ } + + render( + + + + + , + ) + await vi.advanceTimersByTimeAsync(11) + + // The layout effect took care of this one, the passive effect has nothing + // left to do with it + expect(deserializeData).toHaveBeenCalledTimes(1) + + queryClient.clear() + }) + it('should not hydrate queries if state is null', async () => { const queryClient = new QueryClient()