fix(preact-query): hydrate unobserved queries before observers subscribe to avoid a redundant refetch

Author: ostapondo

.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.

packages/preact-query/src/HydrationBoundary.tsx

@@ -7,10 +7,15 @@ 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 { useQueryClient } from './QueryClientProvider'
 
+interface HydrationQueue {
+  queries: DehydratedState['queries']
+  unobserved: DehydratedState['queries']
+}
+
 export interface HydrationBoundaryProps {
   state: DehydratedState | null | undefined
   options?: OmitKeyof<HydrateOptions, 'defaultOptions'> & {
@@ -32,7 +37,7 @@ export const HydrationBoundary = ({
   const client = useQueryClient(queryClient)
 
   const optionsRef = useRef(options)
-  useEffect(() => {
+  useLayoutEffect(() => {
     optionsRef.current = options
   })
 
@@ -51,7 +56,10 @@ 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.
-  const hydrationQueue: DehydratedState['queries'] | undefined = useMemo(() => {
+  //
+  // Queries that no observer is watching are the exception, they are hydrated
+  // as soon as the tree commits, see the layout effect below.
+  const hydrationQueue: HydrationQueue | undefined = useMemo(() => {
     if (state) {
       if (typeof state !== 'object') {
         return
@@ -66,6 +74,7 @@ export const HydrationBoundary = ({
 
       const newQueries: DehydratedState['queries'] = []
       const existingQueries: DehydratedState['queries'] = []
+      const unobservedQueries: DehydratedState['queries'] = []
       for (const dehydratedQuery of queries) {
         const existingQuery = queryCache.get(dehydratedQuery.queryHash)
 
@@ -83,6 +92,10 @@ export const HydrationBoundary = ({
 
           if (hydrationIsNewer) {
             existingQueries.push(dehydratedQuery)
+
+            if (existingQuery.getObserversCount() === 0) {
+              unobservedQueries.push(dehydratedQuery)
+            }
           }
         }
       }
@@ -93,15 +106,36 @@ export const HydrationBoundary = ({
         hydrate(client, { queries: newQueries }, optionsRef.current)
       }
       if (existingQueries.length > 0) {
-        return existingQueries
+        return { queries: existingQueries, unobserved: unobservedQueries }
       }
     }
     return undefined
   }, [client, state])
 
+  // Waiting for a passive effect is too late for a query nobody was observing.
+  // 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.
+  // Nothing was rendering these queries, so putting them in the cache at commit
+  // time cannot change anything on screen, which is the only reason existing
+  // queries wait in the first place.
+  useLayoutEffect(() => {
+    if (hydrationQueue && hydrationQueue.unobserved.length > 0) {
+      hydrate(
+        client,
+        { queries: hydrationQueue.unobserved },
+        optionsRef.current,
+      )
+    }
+  }, [client, hydrationQueue])
+
+  // Observed queries 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. Handing over the whole queue is fine, hydrating the same data a
+  // second time does nothing.
   useEffect(() => {
     if (hydrationQueue) {
-      hydrate(client, { queries: hydrationQueue }, optionsRef.current)
+      hydrate(client, { queries: hydrationQueue.queries }, optionsRef.current)
     }
   }, [client, hydrationQueue])
 

packages/preact-query/src/__tests__/HydrationBoundary.test.tsx

@@ -167,13 +167,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 +237,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 +254,16 @@ describe('Preact hydration', () => {
           </QueryClientProvider>,
         )
 
-        // 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 +312,122 @@ 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 <div>{data}</div>
+    }
+
+    // First visit fetches and caches the data
+    const rendered = render(
+      <QueryClientProvider client={queryClient}>
+        <Page />
+      </QueryClientProvider>,
+    )
+    await vi.advanceTimersByTimeAsync(11)
+    expect(rendered.getByText('client')).toBeInTheDocument()
+
+    // Navigate away, the cached data goes stale while the page is unmounted
+    rendered.rerender(
+      <QueryClientProvider client={queryClient}>
+        <div />
+      </QueryClientProvider>,
+    )
+    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(
+      <QueryClientProvider client={queryClient}>
+        <HydrationBoundary state={dehydratedState}>
+          <Page />
+        </HydrationBoundary>
+      </QueryClientProvider>,
+    )
+
+    // 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 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 <div>{data}</div>
+    }
+
+    function Thrower(): never {
+      throw new Promise(() => {
+        // Never resolve
+      })
+    }
+
+    const rendered = render(
+      <QueryClientProvider client={queryClient}>
+        <Sidebar />
+      </QueryClientProvider>,
+    )
+    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(
+      <QueryClientProvider client={queryClient}>
+        <Sidebar />
+        <Suspense fallback="loading">
+          <HydrationBoundary state={dehydratedState}>
+            <Thrower />
+          </HydrationBoundary>
+        </Suspense>
+      </QueryClientProvider>,
+    )
+
+    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 queries if state is null', async () => {
     const queryClient = new QueryClient()