Revert "fix(core) : sync Observer 'current' properties when optimisti…

…c reading occurs (#5611)"

This reverts commit cefd080.

packages/query-core/src/queryObserver.ts

@@ -227,30 +227,7 @@ export class QueryObserver<
   ): QueryObserverResult<TData, TError> {
     const query = this.#client.getQueryCache().build(this.#client, 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
+    return this.createResult(query, options)
   }
 
   getCurrentResult(): QueryObserverResult<TData, TError> {
@@ -748,25 +725,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<TQueryFnData, TError, TData, TQueryData, TQueryKey>,
-  optimisticResult: QueryObserverResult<TData, TError>,
-) {
-  // 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
-}

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(2)
+    expect(states).toHaveLength(3)
 
     expect(fetched).toBe(false)
 
@@ -354,6 +354,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 () => {

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(6))
+    await waitFor(() => expect(states.length).toBe(7))
 
     expect(states[0]).toMatchObject({
       data: undefined,
@@ -251,7 +251,15 @@ 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,

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

@@ -692,15 +692,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({ 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[3]).toMatchObject({ isPending: false, isSuccess: true })
+    expect(states[4]).toMatchObject({ isPending: false, isSuccess: true })
   })
 
   it('should fetch when refetchOnMount is false and nothing has been fetched yet', async () => {
@@ -1671,7 +1673,7 @@ describe('useQuery', () => {
     act(() => rendered.rerender(<Page count={2} />))
     await waitFor(() => rendered.getByText('error: Error test'))
 
-    await waitFor(() => expect(states.length).toBe(6))
+    await waitFor(() => expect(states.length).toBe(8))
     // Initial
     expect(states[0]).toMatchObject({
       data: undefined,
@@ -1696,30 +1698,46 @@ describe('useQuery', () => {
       error: null,
       isPlaceholderData: true,
     })
-    // New data
+    // Hook state update
     expect(states[3]).toMatchObject({
+      data: 0,
+      isFetching: true,
+      status: 'success',
+      error: null,
+      isPlaceholderData: true,
+    })
+    // New data
+    expect(states[4]).toMatchObject({
       data: 1,
       isFetching: false,
       status: 'success',
       error: null,
       isPlaceholderData: false,
     })
     // rerender Page 2
-    expect(states[4]).toMatchObject({
+    expect(states[5]).toMatchObject({
+      data: 1,
+      isFetching: true,
+      status: 'success',
+      error: null,
+      isPlaceholderData: true,
+    })
+    // Hook state update again
+    expect(states[6]).toMatchObject({
       data: 1,
       isFetching: true,
       status: 'success',
       error: null,
       isPlaceholderData: true,
     })
     // Error
-    expect(states[5]).toMatchObject({
+    expect(states[7]).toMatchObject({
       data: undefined,
       isFetching: false,
       status: 'error',
       isPlaceholderData: false,
     })
-    expect(states[5]!.error).toHaveProperty('message', 'Error test')
+    expect(states[7]?.error).toHaveProperty('message', 'Error test')
   })
 
   it('should not show initial data from next query if placeholderData is set', async () => {
@@ -1764,7 +1782,7 @@ describe('useQuery', () => {
       rendered.getByText('data: 1, count: 1, isFetching: false'),
     )
 
-    expect(states.length).toBe(4)
+    expect(states.length).toBe(5)
 
     // Initial
     expect(states[0]).toMatchObject({
@@ -1787,8 +1805,15 @@ describe('useQuery', () => {
       isSuccess: true,
       isPlaceholderData: false,
     })
-    // New data
+    // Hook state update
     expect(states[3]).toMatchObject({
+      data: 99,
+      isFetching: true,
+      isSuccess: true,
+      isPlaceholderData: false,
+    })
+    // New data
+    expect(states[4]).toMatchObject({
       data: 1,
       isFetching: false,
       isSuccess: true,
@@ -6105,127 +6130,4 @@ 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 (
-        <div>
-          <button name="1" onClick={() => setId(1)}>
-            1
-          </button>
-          <button name="2" onClick={() => setId(2)}>
-            2
-          </button>
-          <span>Rendered Id: {data?.id}</span>
-        </div>
-      )
-    }
-
-    function selector(data: any) {
-      return data.numbers.current
-    }
-
-    const rendered = renderWithClient(queryClient, <Test />)
-    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 (
-        <div>
-          <button name="1" onClick={() => setId(1)}>
-            1
-          </button>
-          <button name="2" onClick={() => setId(2)}>
-            2
-          </button>
-          <span>Rendered Id: {data?.id}</span>
-        </div>
-      )
-    }
-
-    function selector(data: any) {
-      return data.numbers.current
-    }
-
-    const rendered = renderWithClient(queryClient, <Test />)
-    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)
-  })
 })

packages/vue-query/src/__tests__/vueQueryPlugin.test.ts

@@ -313,11 +313,8 @@ describe('VueQueryPlugin', () => {
           vi.fn(),
           new Promise((resolve) => {
             setTimeout(() => {
-              client.setQueryData(['persist1'], () => ({
-                foo1: 'bar1',
-              }))
-              client.setQueryData(['persist2'], () => ({
-                foo2: 'bar2',
+              client.setQueryData(['persist'], () => ({
+                foo: 'bar',
               }))
               resolve()
             }, 0)
@@ -327,19 +324,11 @@ describe('VueQueryPlugin', () => {
 
       const fnSpy = vi.fn()
 
-      const query = useQuery(
-        {
-          queryKey: ['persist1'],
-          queryFn: fnSpy,
-        },
-        customClient,
-      )
-
       const queries = useQueries(
         {
           queries: [
             {
-              queryKey: ['persist2'],
+              queryKey: ['persist'],
               queryFn: fnSpy,
             },
           ],
@@ -348,19 +337,14 @@ 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)
 
       await flushPromises()
 
       expect(customClient.isRestoring.value).toBeFalsy()
-      expect(query.data.value).toStrictEqual({ foo1: 'bar1' })
-      expect(queries.value[0].data).toStrictEqual({ foo2: 'bar2' })
+      expect(queries.value[0].data).toStrictEqual({ foo: 'bar' })
       expect(fnSpy).toHaveBeenCalledTimes(0)
     })
   })