diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index e85973a60d..fffdb8071d 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -240,30 +240,7 @@ export class QueryObserver< ): QueryObserverResult { const query = this.client.getQueryCache().build(this.client, 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 + return this.createResult(query, options) } getCurrentResult(): QueryObserverResult { @@ -793,51 +770,3 @@ 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 0dc4bb843c..6e15c0beae 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(2) + expect(states).toHaveLength(3) expect(fetched).toBe(false) @@ -340,6 +340,9 @@ 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 efe33951ea..849ccc2204 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -914,15 +914,17 @@ 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(4) + expect(states.length).toBe(5) // 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[3]).toMatchObject({ isLoading: false, isSuccess: true }) + expect(states[4]).toMatchObject({ isLoading: false, isSuccess: true }) }) it('should fetch when refetchOnMount is false and nothing has been fetched yet', async () => { @@ -3815,7 +3817,6 @@ describe('useQuery', () => { ) act(() => setPrefetched(true)) } - prefetch() }, []) @@ -6045,7 +6046,6 @@ describe('useQuery', () => { ) } - const rendered = renderWithClient(queryClient, ) const fetchBtn = rendered.getByRole('button', { name: 'refetch' }) await waitFor(() => rendered.getByText('data: 1')) @@ -6083,132 +6083,8 @@ 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) - }) })