diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 9f1dbee919..58c902d580 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -227,7 +227,30 @@ export class QueryObserver< ): QueryObserverResult { const query = this.#client.getQueryCache().build(this.#client, options) - return this.createResult(query, options) + const result = this.createResult(query, options) + + if (shouldAssignObserverCurrentProperties(this, result)) { + // this assigns the optimistic result to the current Observer + // because if the query function changes, useQuery will be performing + // an effect where it would fetch again. + // When the fetch finishes, we perform a deep data cloning in order + // to reuse objects references. This deep data clone is performed against + // the `observer.currentResult.data` property + // When QueryKey changes, we refresh the query and get new `optimistic` + // result, while we leave the `observer.currentResult`, so when new data + // arrives, it finds the old `observer.currentResult` which is related + // to the old QueryKey. Which means that currentResult and selectData are + // out of sync already. + // To solve this, we move the cursor of the currentResult everytime + // an observer reads an optimistic value. + + // When keeping the previous data, the result doesn't change until new + // data arrives. + this.#currentResult = result + this.#currentResultOptions = this.options + this.#currentResultState = this.#currentQuery.state + } + return result } getCurrentResult(): QueryObserverResult { @@ -719,3 +742,25 @@ function isStale( ): boolean { return query.isStaleByTime(options.staleTime) } + +// this function would decide if we will update the observer's 'current' +// properties after an optimistic reading via getOptimisticResult +function shouldAssignObserverCurrentProperties< + TQueryFnData = unknown, + TError = unknown, + TData = TQueryFnData, + TQueryData = TQueryFnData, + TQueryKey extends QueryKey = QueryKey, +>( + observer: QueryObserver, + optimisticResult: QueryObserverResult, +) { + // if the newly created result isn't what the observer is holding as current, + // then we'll need to update the properties as well + if (observer.getCurrentResult() !== optimisticResult) { + return true + } + + // basically, just keep previous properties if nothing changed + return false +} diff --git a/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx b/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx index f1e617ea25..721d9bf3cb 100644 --- a/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx +++ b/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx @@ -339,7 +339,7 @@ describe('PersistQueryClientProvider', () => { await waitFor(() => rendered.getByText('data: null')) await waitFor(() => rendered.getByText('data: hydrated')) - expect(states).toHaveLength(3) + expect(states).toHaveLength(2) expect(fetched).toBe(false) @@ -354,9 +354,6 @@ describe('PersistQueryClientProvider', () => { fetchStatus: 'idle', data: 'hydrated', }) - - // #5443 seems like we get an extra render now ... - expect(states[1]).toStrictEqual(states[2]) }) test('should call onSuccess after successful restoring', async () => { diff --git a/packages/react-query/src/__tests__/useInfiniteQuery.test.tsx b/packages/react-query/src/__tests__/useInfiniteQuery.test.tsx index d2a0c7785a..38b8134c7c 100644 --- a/packages/react-query/src/__tests__/useInfiniteQuery.test.tsx +++ b/packages/react-query/src/__tests__/useInfiniteQuery.test.tsx @@ -213,7 +213,7 @@ describe('useInfiniteQuery', () => { await waitFor(() => rendered.getByText('data: 0-asc')) await waitFor(() => rendered.getByText('isFetching: false')) - await waitFor(() => expect(states.length).toBe(7)) + await waitFor(() => expect(states.length).toBe(6)) expect(states[0]).toMatchObject({ data: undefined, @@ -251,15 +251,7 @@ describe('useInfiniteQuery', () => { isSuccess: true, isPlaceholderData: true, }) - // Hook state update expect(states[5]).toMatchObject({ - data: { pages: ['0-desc', '1-desc'] }, - isFetching: true, - isFetchingNextPage: false, - isSuccess: true, - isPlaceholderData: true, - }) - expect(states[6]).toMatchObject({ data: { pages: ['0-asc'] }, isFetching: false, isFetchingNextPage: false, diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 207bc9dd09..c31a5b8cdf 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -693,17 +693,15 @@ describe('useQuery', () => { // required to make sure no additional renders are happening after data is successfully fetched for the second time await sleep(100) - expect(states.length).toBe(5) + expect(states.length).toBe(4) // First load expect(states[0]).toMatchObject({ isPending: true, isSuccess: false }) // First success expect(states[1]).toMatchObject({ isPending: false, isSuccess: true }) // Remove expect(states[2]).toMatchObject({ isPending: true, isSuccess: false }) - // Hook state update - expect(states[3]).toMatchObject({ isPending: true, isSuccess: false }) // Second success - expect(states[4]).toMatchObject({ isPending: false, isSuccess: true }) + expect(states[3]).toMatchObject({ isPending: false, isSuccess: true }) }) it('should fetch when refetchOnMount is false and nothing has been fetched yet', async () => { @@ -1716,7 +1714,7 @@ describe('useQuery', () => { act(() => rendered.rerender()) await waitFor(() => rendered.getByText('error: Error test')) - await waitFor(() => expect(states.length).toBe(8)) + await waitFor(() => expect(states.length).toBe(6)) // Initial expect(states[0]).toMatchObject({ data: undefined, @@ -1741,16 +1739,8 @@ describe('useQuery', () => { error: null, isPlaceholderData: true, }) - // Hook state update - expect(states[3]).toMatchObject({ - data: 0, - isFetching: true, - status: 'success', - error: null, - isPlaceholderData: true, - }) // New data - expect(states[4]).toMatchObject({ + expect(states[3]).toMatchObject({ data: 1, isFetching: false, status: 'success', @@ -1758,15 +1748,7 @@ describe('useQuery', () => { isPlaceholderData: false, }) // rerender Page 2 - expect(states[5]).toMatchObject({ - data: 1, - isFetching: true, - status: 'success', - error: null, - isPlaceholderData: true, - }) - // Hook state update again - expect(states[6]).toMatchObject({ + expect(states[4]).toMatchObject({ data: 1, isFetching: true, status: 'success', @@ -1774,13 +1756,13 @@ describe('useQuery', () => { isPlaceholderData: true, }) // Error - expect(states[7]).toMatchObject({ + expect(states[5]).toMatchObject({ data: undefined, isFetching: false, status: 'error', isPlaceholderData: false, }) - expect(states[7]?.error).toHaveProperty('message', 'Error test') + expect(states[5]!.error).toHaveProperty('message', 'Error test') }) it('should not show initial data from next query if placeholderData is set', async () => { @@ -1825,7 +1807,7 @@ describe('useQuery', () => { rendered.getByText('data: 1, count: 1, isFetching: false'), ) - expect(states.length).toBe(5) + expect(states.length).toBe(4) // Initial expect(states[0]).toMatchObject({ @@ -1848,15 +1830,8 @@ describe('useQuery', () => { isSuccess: true, isPlaceholderData: false, }) - // Hook state update - expect(states[3]).toMatchObject({ - data: 99, - isFetching: true, - isSuccess: true, - isPlaceholderData: false, - }) // New data - expect(states[4]).toMatchObject({ + expect(states[3]).toMatchObject({ data: 1, isFetching: false, isSuccess: true, @@ -5993,4 +5968,127 @@ describe('useQuery', () => { await waitFor(() => rendered.getByText('status: success')) await waitFor(() => rendered.getByText('data: 1')) }) + it('should reuse same data object reference when queryKey changes back to some cached data', async () => { + const key = queryKey() + const spy = vi.fn() + + async function fetchNumber(id: number) { + await sleep(5) + return { numbers: { current: { id } } } + } + function Test() { + const [id, setId] = React.useState(1) + + const { data } = useQuery({ + select: selector, + queryKey: [key, 'user', id], + queryFn: () => fetchNumber(id), + }) + + React.useEffect(() => { + spy(data) + }, [data]) + + return ( +
+ + + Rendered Id: {data?.id} +
+ ) + } + + function selector(data: any) { + return data.numbers.current + } + + const rendered = renderWithClient(queryClient, ) + expect(spy).toHaveBeenCalledTimes(1) + + spy.mockClear() + await waitFor(() => rendered.getByText('Rendered Id: 1')) + expect(spy).toHaveBeenCalledTimes(1) + + spy.mockClear() + fireEvent.click(rendered.getByRole('button', { name: /2/ })) + await waitFor(() => rendered.getByText('Rendered Id: 2')) + expect(spy).toHaveBeenCalledTimes(2) // called with undefined because id changed + + spy.mockClear() + fireEvent.click(rendered.getByRole('button', { name: /1/ })) + await waitFor(() => rendered.getByText('Rendered Id: 1')) + expect(spy).toHaveBeenCalledTimes(1) + + spy.mockClear() + fireEvent.click(rendered.getByRole('button', { name: /2/ })) + await waitFor(() => rendered.getByText('Rendered Id: 2')) + expect(spy).toHaveBeenCalledTimes(1) + }) + it('should reuse same data object reference when queryKey changes and placeholderData is present', async () => { + const key = queryKey() + const spy = vi.fn() + + async function fetchNumber(id: number) { + await sleep(5) + return { numbers: { current: { id } } } + } + function Test() { + const [id, setId] = React.useState(1) + + const { data } = useQuery({ + select: selector, + queryKey: [key, 'user', id], + queryFn: () => fetchNumber(id), + placeholderData: { numbers: { current: { id: 99 } } }, + }) + + React.useEffect(() => { + spy(data) + }, [data]) + + return ( +
+ + + Rendered Id: {data?.id} +
+ ) + } + + function selector(data: any) { + return data.numbers.current + } + + const rendered = renderWithClient(queryClient, ) + expect(spy).toHaveBeenCalledTimes(1) + + spy.mockClear() + await waitFor(() => rendered.getByText('Rendered Id: 99')) + await waitFor(() => rendered.getByText('Rendered Id: 1')) + expect(spy).toHaveBeenCalledTimes(1) + + spy.mockClear() + fireEvent.click(rendered.getByRole('button', { name: /2/ })) + await waitFor(() => rendered.getByText('Rendered Id: 99')) + await waitFor(() => rendered.getByText('Rendered Id: 2')) + expect(spy).toHaveBeenCalledTimes(2) // called with undefined because id changed + + spy.mockClear() + fireEvent.click(rendered.getByRole('button', { name: /1/ })) + await waitFor(() => rendered.getByText('Rendered Id: 1')) + expect(spy).toHaveBeenCalledTimes(1) + + spy.mockClear() + fireEvent.click(rendered.getByRole('button', { name: /2/ })) + await waitFor(() => rendered.getByText('Rendered Id: 2')) + expect(spy).toHaveBeenCalledTimes(1) + }) }) diff --git a/packages/vue-query/src/__tests__/vueQueryPlugin.test.ts b/packages/vue-query/src/__tests__/vueQueryPlugin.test.ts index 7ef3bed2b6..c6ebb6815f 100644 --- a/packages/vue-query/src/__tests__/vueQueryPlugin.test.ts +++ b/packages/vue-query/src/__tests__/vueQueryPlugin.test.ts @@ -313,8 +313,11 @@ describe('VueQueryPlugin', () => { vi.fn(), new Promise((resolve) => { setTimeout(() => { - client.setQueryData(['persist'], () => ({ - foo: 'bar', + client.setQueryData(['persist1'], () => ({ + foo1: 'bar1', + })) + client.setQueryData(['persist2'], () => ({ + foo2: 'bar2', })) resolve() }, 0) @@ -324,11 +327,19 @@ describe('VueQueryPlugin', () => { const fnSpy = vi.fn() + const query = useQuery( + { + queryKey: ['persist1'], + queryFn: fnSpy, + }, + customClient, + ) + const queries = useQueries( { queries: [ { - queryKey: ['persist'], + queryKey: ['persist2'], queryFn: fnSpy, }, ], @@ -337,6 +348,10 @@ describe('VueQueryPlugin', () => { ) expect(customClient.isRestoring.value).toBeTruthy() + + expect(query.isFetching.value).toBeFalsy() + expect(query.data.value).toStrictEqual(undefined) + expect(queries.value[0].isFetching).toBeFalsy() expect(queries.value[0].data).toStrictEqual(undefined) expect(fnSpy).toHaveBeenCalledTimes(0) @@ -344,7 +359,8 @@ describe('VueQueryPlugin', () => { await flushPromises() expect(customClient.isRestoring.value).toBeFalsy() - expect(queries.value[0].data).toStrictEqual({ foo: 'bar' }) + expect(query.data.value).toStrictEqual({ foo1: 'bar1' }) + expect(queries.value[0].data).toStrictEqual({ foo2: 'bar2' }) expect(fnSpy).toHaveBeenCalledTimes(0) }) }) diff --git a/packages/vue-query/src/useQueries.ts b/packages/vue-query/src/useQueries.ts index 47651ff623..1b479ab0f8 100644 --- a/packages/vue-query/src/useQueries.ts +++ b/packages/vue-query/src/useQueries.ts @@ -214,7 +214,10 @@ export function useQueries< defaultedQueries.value, options as QueriesObserverOptions, ) - state.value = observer.getCurrentResult() + const [, getCombinedResultPersisted] = observer.getOptimisticResult( + defaultedQueries.value, + ) + state.value = getCombinedResultPersisted() }, { deep: true }, )