Repository: tannerlinsley/react-query (ts)

Issue:
Possible inconsistency with `isError` and `error` when using `keepPreviousData: true`
- Turn on `keepPreviousData: true`
- Template using `isError`, not `error`, to detect an error state
- Fire a query that resolves and displays normally
- Fire a query that fails and introduces an error state
- The `isError` state is `false`, but there is clearly an `error` present.

I *think* the desired effect is that if an `error` is encountered in the *new/latest* query, the status should not be `success` with the previous data, but `error` with the new query.

Codesandbox: https://codesandbox.io/s/optimistic-hooks-n5v12?file=/src/index.js

Pull request description:
Bugfix/keep previous data error
closes #1659

Patch:
diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts
index cbe44247b..e40d5cc76 100644
--- a/src/core/queryObserver.ts
+++ b/src/core/queryObserver.ts
@@ -368,7 +368,8 @@ export class QueryObserver<
     if (
       this.options.keepPreviousData &&
       !state.dataUpdateCount &&
-      this.previousQueryResult?.isSuccess
+      this.previousQueryResult?.isSuccess &&
+      status !== 'error'
     ) {
       data = this.previousQueryResult.data
       dataUpdatedAt = this.previousQueryResult.dataUpdatedAt


Test patch:
diff --git a/src/react/tests/useQuery.test.tsx b/src/react/tests/useQuery.test.tsx
index 3848e5a87..67d21e6ef 100644
--- a/src/react/tests/useQuery.test.tsx
+++ b/src/react/tests/useQuery.test.tsx
@@ -1123,6 +1123,112 @@ describe('useQuery', () => {
     })
   })
 
+  it('should transition to error state when keepPreviousData is set', async () => {
+    const key = queryKey()
+    const consoleMock = mockConsoleError()
+    const states: UseQueryResult<number>[] = []
+
+    function Page({ count }: { count: number }) {
+      const state = useQuery<number, Error>(
+        [key, count],
+        async () => {
+          if (count === 2) {
+            throw new Error('Error test')
+          }
+          return Promise.resolve(count)
+        },
+        {
+          retry: false,
+          keepPreviousData: true,
+        }
+      )
+
+      states.push(state)
+
+      return (
+        <div>
+          <h1>data: {state.data}</h1>
+          <h2>error: {state.error?.message}</h2>
+        </div>
+      )
+    }
+
+    const rendered = renderWithClient(queryClient, <Page count={0} />)
+    await waitFor(() => rendered.getByText('data: 0'))
+    act(() => rendered.rerender(<Page count={1} />))
+    await waitFor(() => rendered.getByText('data: 1'))
+    act(() => rendered.rerender(<Page count={2} />))
+    await waitFor(() => rendered.getByText('error: Error test'))
+
+    expect(states.length).toBe(8)
+    // Initial
+    expect(states[0]).toMatchObject({
+      data: undefined,
+      isFetching: true,
+      status: 'loading',
+      error: null,
+      isPreviousData: false,
+    })
+    // Fetched
+    expect(states[1]).toMatchObject({
+      data: 0,
+      isFetching: false,
+      status: 'success',
+      error: null,
+      isPreviousData: false,
+    })
+    // rerender Page 1
+    expect(states[2]).toMatchObject({
+      data: 0,
+      isFetching: true,
+      status: 'success',
+      error: null,
+      isPreviousData: true,
+    })
+    // Hook state update
+    expect(states[3]).toMatchObject({
+      data: 0,
+      isFetching: true,
+      status: 'success',
+      error: null,
+      isPreviousData: true,
+    })
+    // New data
+    expect(states[4]).toMatchObject({
+      data: 1,
+      isFetching: false,
+      status: 'success',
+      error: null,
+      isPreviousData: false,
+    })
+    // rerender Page 2
+    expect(states[5]).toMatchObject({
+      data: 1,
+      isFetching: true,
+      status: 'success',
+      error: null,
+      isPreviousData: true,
+    })
+    // Hook state update again
+    expect(states[6]).toMatchObject({
+      data: 1,
+      isFetching: true,
+      status: 'success',
+      error: null,
+      isPreviousData: true,
+    })
+    // Error
+    expect(states[7]).toMatchObject({
+      data: undefined,
+      isFetching: false,
+      status: 'error',
+      isPreviousData: false,
+    })
+    expect(states[7].error).toHaveProperty('message', 'Error test')
+
+    consoleMock.mockRestore()
+  })
+
   it('should not show initial data from next query if keepPreviousData is set', async () => {
     const key = queryKey()
     const states: UseQueryResult<number>[] = []
diff --git a/src/react/tests/utils.tsx b/src/react/tests/utils.tsx
index 17c3d07ea..609a9d164 100644
--- a/src/react/tests/utils.tsx
+++ b/src/react/tests/utils.tsx
@@ -4,7 +4,16 @@ import React from 'react'
 import { QueryClient, QueryClientProvider } from '../..'
 
 export function renderWithClient(client: QueryClient, ui: React.ReactElement) {
-  return render(<QueryClientProvider client={client}>{ui}</QueryClientProvider>)
+  const { rerender, ...result } = render(
+    <QueryClientProvider client={client}>{ui}</QueryClientProvider>
+  )
+  return {
+    ...result,
+    rerender: (rerenderUi: React.ReactElement) =>
+      rerender(
+        <QueryClientProvider client={client}>{rerenderUi}</QueryClientProvider>
+      ),
+  }
 }
 
 export function mockVisibilityState(value: string) {