diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 123996e572..4e4ace8f8b 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -240,7 +240,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, options)) { + // 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 { @@ -764,3 +787,51 @@ 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, + options: DefaultedQueryObserverOptions< + TQueryFnData, + TError, + TData, + TQueryData, + TQueryKey + >, +) { + // it is important to keep this condition like this for three reasons: + // 1. It will get removed in the v5 + // 2. it reads: don't update the properties if we want to keep the previous + // data. + // 3. The opposite condition (!options.keepPreviousData) would fallthrough + // and will result in a bad decision + if (options.keepPreviousData) { + return false + } + + // this means we want to put some placeholder data when pending and queryKey + // changed. + if (options.placeholderData !== undefined) { + // re-assign properties only if current data is placeholder data + // which means that data did not arrive yet, so, if there is some cached data + // we need to "prepare" to receive it + return optimisticResult.isPlaceholderData + } + + // 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 3aa840221f..bce3257568 100644 --- a/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx +++ b/packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx @@ -325,7 +325,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) @@ -340,9 +340,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__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 2fb62e5b45..b5550d8858 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -914,17 +914,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({ isLoading: true, isSuccess: false }) // First success expect(states[1]).toMatchObject({ isLoading: false, isSuccess: true }) // Remove expect(states[2]).toMatchObject({ isLoading: true, isSuccess: false }) - // Hook state update - expect(states[3]).toMatchObject({ isLoading: true, isSuccess: false }) // Second success - expect(states[4]).toMatchObject({ isLoading: false, isSuccess: true }) + expect(states[3]).toMatchObject({ isLoading: false, isSuccess: true }) }) it('should fetch when refetchOnMount is false and nothing has been fetched yet', async () => { @@ -3650,6 +3648,7 @@ describe('useQuery', () => { ) act(() => setPrefetched(true)) } + prefetch() }, []) @@ -5879,6 +5878,7 @@ describe('useQuery', () => { ) } + const rendered = renderWithClient(queryClient, ) const fetchBtn = rendered.getByRole('button', { name: 'refetch' }) await waitFor(() => rendered.getByText('data: 1')) @@ -5916,8 +5916,132 @@ describe('useQuery', () => { ) } + const rendered = renderWithClient(queryClient, ) 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 spy = jest.fn() + const key = queryKey() + + 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 spy = jest.fn() + const key = queryKey() + + 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) + }) })