fix(query-core): ensure combine re-executes after cache restoration with memoized combine (#9592)

* fix(query-core): ensure combine re-executes after cache restoration with memoized combine

* fix(query-core): add error field to result change detection

* test(query-core): add test for QueriesObserver early return notification

* test(query-core): improve test for QueriesObserver early return notification

* refactor(query-core): use shallowEqualObjects for result comparison

* refactor(query-core): improve setQueries flow and fix persist/restore issue with memoized combine

* refactor(query-core): improve setQueries flow and remove log

* test(react-query): update useQueries expectations for metadata change detection

* test(react-query): fix test comments

* test(react-query): add test for stable combine optimization on unrelated re-renders

---------

Co-authored-by: Dominik Dorfmeister <[email protected]>

Author: joseph0926

packages/query-core/src/__tests__/queriesObserver.test.tsx

@@ -347,4 +347,51 @@ describe('queriesObserver', () => {
     expect(queryFn1).toHaveBeenCalledTimes(1)
     expect(queryFn2).toHaveBeenCalledTimes(1)
   })
+
+  test('should notify when results change during early return', async () => {
+    const key1 = queryKey()
+    const key2 = queryKey()
+    const queryFn1 = vi.fn().mockReturnValue(1)
+    const queryFn2 = vi.fn().mockReturnValue(2)
+
+    queryClient.setQueryData(key1, 1)
+    queryClient.setQueryData(key2, 2)
+
+    const observer = new QueriesObserver(queryClient, [
+      { queryKey: key1, queryFn: queryFn1 },
+      { queryKey: key2, queryFn: queryFn2 },
+    ])
+
+    const results: Array<Array<QueryObserverResult>> = []
+    results.push(observer.getCurrentResult())
+
+    const onUpdate = vi.fn((result: Array<QueryObserverResult>) => {
+      results.push(result)
+    })
+    const unsubscribe = observer.subscribe(onUpdate)
+    const baseline = results.length
+
+    observer.setQueries([
+      {
+        queryKey: key1,
+        queryFn: queryFn1,
+        select: (d: any) => d + 100,
+      },
+      {
+        queryKey: key2,
+        queryFn: queryFn2,
+        select: (d: any) => d + 100,
+      },
+    ])
+
+    await vi.advanceTimersByTimeAsync(0)
+
+    unsubscribe()
+
+    expect(results.length).toBeGreaterThan(baseline)
+    expect(results[results.length - 1]).toMatchObject([
+      { status: 'success', data: 101 },
+      { status: 'success', data: 102 },
+    ])
+  })
 })

packages/query-core/src/queriesObserver.ts

@@ -1,7 +1,7 @@
 import { notifyManager } from './notifyManager'
 import { QueryObserver } from './queryObserver'
 import { Subscribable } from './subscribable'
-import { replaceEqualDeep } from './utils'
+import { replaceEqualDeep, shallowEqualObjects } from './utils'
 import type {
   DefaultedQueryObserverOptions,
   QueryObserverOptions,
@@ -122,26 +122,34 @@ export class QueriesObserver<
         (observer, index) => observer !== prevObservers[index],
       )
 
-      if (prevObservers.length === newObservers.length && !hasIndexChange) {
-        return
-      }
+      const hasResultChange =
+        prevObservers.length === newObservers.length && !hasIndexChange
+          ? newResult.some((result, index) => {
+              const prev = this.#result[index]
+              return !prev || !shallowEqualObjects(result, prev)
+            })
+          : true
 
-      this.#observers = newObservers
-      this.#result = newResult
+      if (!hasIndexChange && !hasResultChange) return
 
-      if (!this.hasListeners()) {
-        return
+      if (hasIndexChange) {
+        this.#observers = newObservers
       }
 
-      difference(prevObservers, newObservers).forEach((observer) => {
-        observer.destroy()
-      })
+      this.#result = newResult
 
-      difference(newObservers, prevObservers).forEach((observer) => {
-        observer.subscribe((result) => {
-          this.#onUpdate(observer, result)
+      if (!this.hasListeners()) return
+
+      if (hasIndexChange) {
+        difference(prevObservers, newObservers).forEach((observer) => {
+          observer.destroy()
         })
-      })
+        difference(newObservers, prevObservers).forEach((observer) => {
+          observer.subscribe((result) => {
+            this.#onUpdate(observer, result)
+          })
+        })
+      }
 
       this.#notify()
     })

packages/react-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx

@@ -0,0 +1,134 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+import { render, waitFor } from '@testing-library/react'
+import * as React from 'react'
+import { QueryClient, useQueries } from '@tanstack/react-query'
+import { PersistQueryClientProvider } from '@tanstack/react-query-persist-client'
+import type {
+  PersistedClient,
+  Persister,
+} from '@tanstack/query-persist-client-core'
+import type { QueryObserverResult } from '@tanstack/react-query'
+
+describe('useQueries with persist and memoized combine', () => {
+  const storage: { [key: string]: string } = {}
+
+  beforeEach(() => {
+    Object.defineProperty(window, 'localStorage', {
+      value: {
+        getItem: (key: string) => storage[key] || null,
+        setItem: (key: string, value: string) => {
+          storage[key] = value
+        },
+        removeItem: (key: string) => {
+          delete storage[key]
+        },
+        clear: () => {
+          Object.keys(storage).forEach((key) => delete storage[key])
+        },
+      },
+      writable: true,
+    })
+  })
+
+  afterEach(() => {
+    Object.keys(storage).forEach((key) => delete storage[key])
+  })
+
+  it('should update UI when combine is memoized with persist', async () => {
+    const queryClient = new QueryClient({
+      defaultOptions: {
+        queries: {
+          staleTime: 30_000,
+          gcTime: 1000 * 60 * 60 * 24,
+        },
+      },
+    })
+
+    const persister: Persister = {
+      persistClient: (client: PersistedClient) => {
+        storage['REACT_QUERY_OFFLINE_CACHE'] = JSON.stringify(client)
+        return Promise.resolve()
+      },
+      restoreClient: async () => {
+        const stored = storage['REACT_QUERY_OFFLINE_CACHE']
+        if (stored) {
+          await new Promise((resolve) => setTimeout(resolve, 10))
+          return JSON.parse(stored) as PersistedClient
+        }
+        return undefined
+      },
+      removeClient: () => {
+        delete storage['REACT_QUERY_OFFLINE_CACHE']
+        return Promise.resolve()
+      },
+    }
+
+    const persistedData: PersistedClient = {
+      timestamp: Date.now(),
+      buster: '',
+      clientState: {
+        mutations: [],
+        queries: [1, 2, 3].map((id) => ({
+          queryHash: `["post",${id}]`,
+          queryKey: ['post', id],
+          state: {
+            data: id,
+            dataUpdateCount: 1,
+            dataUpdatedAt: Date.now() - 1000,
+            error: null,
+            errorUpdateCount: 0,
+            errorUpdatedAt: 0,
+            fetchFailureCount: 0,
+            fetchFailureReason: null,
+            fetchMeta: null,
+            isInvalidated: false,
+            status: 'success' as const,
+            fetchStatus: 'idle' as const,
+          },
+        })),
+      },
+    }
+
+    storage['REACT_QUERY_OFFLINE_CACHE'] = JSON.stringify(persistedData)
+
+    function TestComponent() {
+      const combinedQueries = useQueries({
+        queries: [1, 2, 3].map((id) => ({
+          queryKey: ['post', id],
+          queryFn: () => Promise.resolve(id),
+          staleTime: 30_000,
+        })),
+        combine: React.useCallback(
+          (results: Array<QueryObserverResult<number, Error>>) => ({
+            data: results.map((r) => r.data),
+            isPending: results.some((r) => r.isPending),
+          }),
+          [],
+        ),
+      })
+
+      return (
+        <div>
+          <div data-testid="pending">{String(combinedQueries.isPending)}</div>
+          <div data-testid="data">
+            {combinedQueries.data.filter((d) => d !== undefined).join(',')}
+          </div>
+        </div>
+      )
+    }
+
+    const { getByTestId } = render(
+      <PersistQueryClientProvider
+        client={queryClient}
+        persistOptions={{ persister }}
+      >
+        <TestComponent />
+      </PersistQueryClientProvider>,
+    )
+
+    await waitFor(() => {
+      expect(getByTestId('pending').textContent).toBe('false')
+      expect(getByTestId('data').textContent).toBe('1,2,3')
+    })
+  })
+})

packages/react-query/src/__tests__/useQueries.test.tsx

@@ -1210,7 +1210,7 @@ describe('useQueries', () => {
 
     const length = results.length
 
-    expect([4, 5]).toContain(results.length)
+    expect([4, 5, 6]).toContain(results.length)
 
     expect(results[results.length - 1]).toStrictEqual({
       combined: true,
@@ -1379,8 +1379,8 @@ describe('useQueries', () => {
 
     fireEvent.click(rendered.getByRole('button', { name: /rerender/i }))
 
-    // no increase because just a re-render
-    expect(spy).toHaveBeenCalledTimes(3)
+    // one extra call due to recomputing the combined result on rerender
+    expect(spy).toHaveBeenCalledTimes(4)
 
     value = 1
 
@@ -1391,8 +1391,8 @@ describe('useQueries', () => {
       rendered.getByText('data: true first result:1,second result:1'),
     ).toBeInTheDocument()
 
-    // two value changes = two re-renders
-    expect(spy).toHaveBeenCalledTimes(5)
+    // refetch with new values triggers: both pending -> one pending -> both resolved
+    expect(spy).toHaveBeenCalledTimes(7)
   })
 
   it('should re-run combine if the functional reference changes', async () => {
@@ -1658,4 +1658,87 @@ describe('useQueries', () => {
       ),
     ).toBeInTheDocument()
   })
+
+  it('should not re-run stable combine on unrelated re-render', async () => {
+    const key1 = queryKey()
+    const key2 = queryKey()
+
+    const client = new QueryClient()
+
+    const spy = vi.fn()
+
+    function Page() {
+      const [unrelatedState, setUnrelatedState] = React.useState(0)
+
+      const queries = useQueries(
+        {
+          queries: [
+            {
+              queryKey: key1,
+              queryFn: async () => {
+                await sleep(10)
+                return 'first result'
+              },
+            },
+            {
+              queryKey: key2,
+              queryFn: async () => {
+                await sleep(20)
+                return 'second result'
+              },
+            },
+          ],
+          combine: React.useCallback((results: Array<QueryObserverResult>) => {
+            const result = {
+              combined: true,
+              res: results.map((res) => res.data).join(','),
+            }
+            spy(result)
+            return result
+          }, []),
+        },
+        client,
+      )
+
+      return (
+        <div>
+          <div>
+            data: {String(queries.combined)} {queries.res}
+          </div>
+          <div>unrelated: {unrelatedState}</div>
+          <button onClick={() => setUnrelatedState((s) => s + 1)}>
+            increment
+          </button>
+        </div>
+      )
+    }
+
+    const rendered = render(<Page />)
+
+    await vi.advanceTimersByTimeAsync(21)
+    expect(
+      rendered.getByText('data: true first result,second result'),
+    ).toBeInTheDocument()
+
+    // initial renders: both pending, one pending, both resolved
+    expect(spy).toHaveBeenCalledTimes(3)
+
+    fireEvent.click(rendered.getByRole('button', { name: /increment/i }))
+
+    await vi.advanceTimersByTimeAsync(0)
+
+    expect(rendered.getByText('unrelated: 1')).toBeInTheDocument()
+
+    // combine should NOT re-run for unrelated re-render with stable reference
+    expect(spy).toHaveBeenCalledTimes(3)
+
+    fireEvent.click(rendered.getByRole('button', { name: /increment/i }))
+
+    await vi.advanceTimersByTimeAsync(0)
+
+    expect(rendered.getByText('unrelated: 2')).toBeInTheDocument()
+
+    // still no extra calls to combine
+    expect(spy).toHaveBeenCalledTimes(3)
+  })
 })