fix(core): initialData should take precedence over keepPreviousData (#4422)

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

Author: TkDodo

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,

packages/query-core/src/queryObserver.ts

@@ -468,7 +468,7 @@ export class QueryObserver<
     // Keep previous data if needed
     if (
       options.keepPreviousData &&
-      !state.dataUpdateCount &&
+      !state.dataUpdatedAt &&
       prevQueryResult?.isSuccess &&
       status !== 'error'
     ) {

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()

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({