From 6ea93e740a9d4d1f7e21e6fcba6de8b1ccf7c9c4 Mon Sep 17 00:00:00 2001
From: Dominik Dorfmeister <office@dorfmeister.cc>
Date: Tue, 1 Nov 2022 07:27:42 +0100
Subject: [PATCH] fix(core): initialData should take precedence over
 keepPreviousData

previousData is data that we want to show from a previous observer if we don't have any good data in the cache already. InitialData is always considered good data, just as if it were data that has been fetched. It might be potentially stale, but that's something that can be controlled via staleTime and initialDataUpdatedAt.

So that means we shouldn't show previousData if we also have initialData
---
 packages/query-core/src/query.ts              |  6 +-
 packages/query-core/src/queryObserver.ts      |  2 +-
 .../src/__tests__/useQuery.test.tsx           | 93 ++++++++++++++++---
 .../src/__tests__/createQuery.test.tsx        | 30 ++++--
 4 files changed, 104 insertions(+), 27 deletions(-)

diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts
index 25364b9b18..d26d50a3e5 100644
--- a/packages/query-core/src/query.ts
+++ b/packages/query-core/src/query.ts
@@ -600,16 +600,14 @@ function getDefaultState<
       ? (options.initialData as InitialDataFunction<TData>)()
       : options.initialData
 
-  const hasInitialData = typeof options.initialData !== 'undefined'
+  const hasData = typeof data !== 'undefined'
 
-  const initialDataUpdatedAt = hasInitialData
+  const initialDataUpdatedAt = hasData
     ? typeof options.initialDataUpdatedAt === 'function'
       ? (options.initialDataUpdatedAt as () => number | undefined)()
       : options.initialDataUpdatedAt
     : 0
 
-  const hasData = typeof data !== 'undefined'
-
   return {
     data,
     dataUpdateCount: 0,
diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts
index fd16a00c60..971a51c229 100644
--- a/packages/query-core/src/queryObserver.ts
+++ b/packages/query-core/src/queryObserver.ts
@@ -457,7 +457,7 @@ export class QueryObserver<
     // Keep previous data if needed
     if (
       options.keepPreviousData &&
-      !state.dataUpdateCount &&
+      !state.dataUpdatedAt &&
       prevQueryResult?.isSuccess &&
       status !== 'error'
     ) {
diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx
index 8e6045dca4..c21551b2d3 100644
--- a/packages/react-query/src/__tests__/useQuery.test.tsx
+++ b/packages/react-query/src/__tests__/useQuery.test.tsx
@@ -1769,18 +1769,30 @@ describe('useQuery', () => {
 
       states.push(state)
 
-      React.useEffect(() => {
-        setActTimeout(() => {
-          setCount(1)
-        }, 20)
-      }, [])
-
-      return null
+      return (
+        <div>
+          <h1>
+            data: {state.data}, count: {count}, isFetching:{' '}
+            {String(state.isFetching)}
+          </h1>
+          <button onClick={() => setCount(1)}>inc</button>
+        </div>
+      )
     }
 
-    renderWithClient(queryClient, <Page />)
+    const rendered = renderWithClient(queryClient, <Page />)
 
-    await waitFor(() => expect(states.length).toBe(5))
+    await waitFor(() =>
+      rendered.getByText('data: 0, count: 0, isFetching: false'),
+    )
+
+    fireEvent.click(rendered.getByRole('button', { name: 'inc' }))
+
+    await waitFor(() =>
+      rendered.getByText('data: 1, count: 1, isFetching: false'),
+    )
+
+    expect(states.length).toBe(5)
 
     // Initial
     expect(states[0]).toMatchObject({
@@ -1798,17 +1810,17 @@ describe('useQuery', () => {
     })
     // Set state
     expect(states[2]).toMatchObject({
-      data: 0,
+      data: 99,
       isFetching: true,
       isSuccess: true,
-      isPreviousData: true,
+      isPreviousData: false,
     })
     // Hook state update
     expect(states[3]).toMatchObject({
-      data: 0,
+      data: 99,
       isFetching: true,
       isSuccess: true,
-      isPreviousData: true,
+      isPreviousData: false,
     })
     // New data
     expect(states[4]).toMatchObject({
@@ -3733,6 +3745,61 @@ describe('useQuery', () => {
     expect(results[1]).toMatchObject({ data: 1, isFetching: false })
   })
 
+  it('should show the correct data when switching keys with initialData, keepPreviousData & staleTime', async () => {
+    const key = queryKey()
+
+    const ALL_TODOS = [
+      { name: 'todo A', priority: 'high' },
+      { name: 'todo B', priority: 'medium' },
+    ]
+
+    const initialTodos = ALL_TODOS
+
+    function Page() {
+      const [filter, setFilter] = React.useState('')
+      const { data: todos } = useQuery(
+        [...key, filter],
+        async () => {
+          return ALL_TODOS.filter((todo) =>
+            filter ? todo.priority === filter : true,
+          )
+        },
+        {
+          initialData() {
+            return filter === '' ? initialTodos : undefined
+          },
+          keepPreviousData: true,
+          staleTime: 5000,
+        },
+      )
+
+      return (
+        <div>
+          Current Todos, filter: {filter || 'all'}
+          <hr />
+          <button onClick={() => setFilter('')}>All</button>
+          <button onClick={() => setFilter('high')}>High</button>
+          <ul>
+            {(todos ?? []).map((todo) => (
+              <li key={todo.name}>
+                {todo.name} - {todo.priority}
+              </li>
+            ))}
+          </ul>
+        </div>
+      )
+    }
+
+    const rendered = renderWithClient(queryClient, <Page />)
+
+    await waitFor(() => rendered.getByText('Current Todos, filter: all'))
+
+    fireEvent.click(rendered.getByRole('button', { name: /high/i }))
+    await waitFor(() => rendered.getByText('Current Todos, filter: high'))
+    fireEvent.click(rendered.getByRole('button', { name: /all/i }))
+    await waitFor(() => rendered.getByText('todo B - medium'))
+  })
+
   // // See https://github.com/tannerlinsley/react-query/issues/214
   it('data should persist when enabled is changed to false', async () => {
     const key = queryKey()
diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx
index 70736a9eba..6509af7eef 100644
--- a/packages/solid-query/src/__tests__/createQuery.test.tsx
+++ b/packages/solid-query/src/__tests__/createQuery.test.tsx
@@ -1993,13 +1993,15 @@ describe('createQuery', () => {
         states.push({ ...state })
       })
 
-      createEffect(() => {
-        setActTimeout(() => {
-          setCount(1)
-        }, 20)
-      })
-
-      return null
+      return (
+        <div>
+          <h1>
+            data: {state.data}, count: {count}, isFetching:{' '}
+            {String(state.isFetching)}
+          </h1>
+          <button onClick={() => setCount(1)}>inc</button>
+        </div>
+      )
     }
 
     render(() => (
@@ -2008,6 +2010,16 @@ describe('createQuery', () => {
       </QueryClientProvider>
     ))
 
+    await waitFor(() =>
+      screen.getByText('data: 0, count: 0, isFetching: false'),
+    )
+
+    fireEvent.click(screen.getByRole('button', { name: 'inc' }))
+
+    await waitFor(() =>
+      screen.getByText('data: 1, count: 1, isFetching: false'),
+    )
+
     await waitFor(() => expect(states.length).toBe(4))
 
     // Initial
@@ -2026,10 +2038,10 @@ describe('createQuery', () => {
     })
     // Set state
     expect(states[2]).toMatchObject({
-      data: 0,
+      data: 99,
       isFetching: true,
       isSuccess: true,
-      isPreviousData: true,
+      isPreviousData: false,
     })
     // New data
     expect(states[3]).toMatchObject({