Repository: tannerlinsley/react-query
Pull request #1489 (2020-12-19 13:37:11)

fix: make sure queries are retried after an error boundary reset
This PR contains two fixes:
- Always set query in `loading` state when fetching and no data is cached.
- Make sure components suspend and error when needed.

Fixes https://github.com/tannerlinsley/react-query/issues/1471

vercel[bot]: This pull request is being automatically deployed with Vercel ([learn more](https://vercel.link/github-learn-more)).  
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: [https://vercel.com/tannerlinsley/react-query/d51kw0nsy](https://vercel.com/tannerlinsley/react-query/d51kw0nsy)  
✅ Preview: [https://react-query-git-feature-suspense-fix.tannerlinsley.vercel.app](https://react-query-git-feature-suspense-fix.tannerlinsley.vercel.app)


alexandernanberg: This would fix #1253 afaict

Patch:
diff --git a/src/core/query.ts b/src/core/query.ts
index c44a12b64..a49281368 100644
--- a/src/core/query.ts
+++ b/src/core/query.ts
@@ -495,7 +495,7 @@ export class Query<
           fetchMeta: action.meta ?? null,
           isFetching: true,
           isPaused: false,
-          status: state.status === 'idle' ? 'loading' : state.status,
+          status: !state.dataUpdatedAt ? 'loading' : state.status,
         }
       case 'success':
         return {
@@ -514,12 +514,22 @@ export class Query<
         const error = action.error as unknown
 
         if (isCancelledError(error) && error.revert) {
+          let previousStatus: QueryStatus
+
+          if (!state.dataUpdatedAt && !state.errorUpdatedAt) {
+            previousStatus = 'idle'
+          } else if (state.dataUpdatedAt > state.errorUpdatedAt) {
+            previousStatus = 'success'
+          } else {
+            previousStatus = 'error'
+          }
+
           return {
             ...state,
             fetchFailureCount: 0,
             isFetching: false,
             isPaused: false,
-            status: state.status === 'loading' ? 'idle' : state.status,
+            status: previousStatus,
           }
         }
 
diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts
index a9e998ff6..cbe44247b 100644
--- a/src/core/queryObserver.ts
+++ b/src/core/queryObserver.ts
@@ -96,7 +96,12 @@ export class QueryObserver<
 
   willLoadOnMount(): boolean {
     return (
-      this.options.enabled !== false && !this.currentQuery.state.dataUpdatedAt
+      this.options.enabled !== false &&
+      !this.currentQuery.state.dataUpdatedAt &&
+      !(
+        this.currentQuery.state.status === 'error' &&
+        this.options.retryOnMount === false
+      )
     )
   }
 
@@ -354,7 +359,7 @@ export class QueryObserver<
     // Optimistically set status to loading if we will start fetching
     if (willFetch) {
       isFetching = true
-      if (status === 'idle') {
+      if (!dataUpdatedAt) {
         status = 'loading'
       }
     }
diff --git a/src/core/types.ts b/src/core/types.ts
index f9d95553a..00b06fbe9 100644
--- a/src/core/types.ts
+++ b/src/core/types.ts
@@ -127,6 +127,11 @@ export interface QueryObserverOptions<
    * Defaults to `true`.
    */
   refetchOnMount?: boolean | 'always'
+  /**
+   * If set to `false`, the query will not be retried on mount if it contains an error.
+   * Defaults to `true`.
+   */
+  retryOnMount?: boolean
   /**
    * If set, the component will only re-render if any of the listed properties change.
    * When set to `['data', 'error']`, the component will only re-render when the `data` or `error` properties change.
diff --git a/src/react/useBaseQuery.ts b/src/react/useBaseQuery.ts
index 3aa1b53c0..efa0e7742 100644
--- a/src/react/useBaseQuery.ts
+++ b/src/react/useBaseQuery.ts
@@ -34,13 +34,17 @@ export function useBaseQuery<TQueryFnData, TError, TData, TQueryData>(
     )
   }
 
-  // Always set stale time when using suspense to prevent
-  // fetching again when directly re-mounting after suspense
-  if (
-    defaultedOptions.suspense &&
-    typeof defaultedOptions.staleTime !== 'number'
-  ) {
-    defaultedOptions.staleTime = 1000
+  if (defaultedOptions.suspense) {
+    // Always set stale time when using suspense to prevent
+    // fetching again when directly re-mounting after suspense
+    if (typeof defaultedOptions.staleTime !== 'number') {
+      defaultedOptions.staleTime = 1000
+    }
+
+    // Prevent retrying failed query if the error boundary has not been reset yet
+    if (!errorResetBoundary.isReset()) {
+      defaultedOptions.retryOnMount = false
+    }
   }
 
   // Create query observer
@@ -54,13 +58,14 @@ export function useBaseQuery<TQueryFnData, TError, TData, TQueryData>(
     observer.setOptions(defaultedOptions)
   }
 
-  const [, rerender] = React.useState({})
   const currentResult = observer.getCurrentResult()
 
   // Remember latest result to prevent redundant renders
   const latestResultRef = React.useRef(currentResult)
   latestResultRef.current = currentResult
 
+  const [, rerender] = React.useState({})
+
   // Subscribe to the observer
   React.useEffect(() => {
     errorResetBoundary.clearReset()
@@ -75,19 +80,15 @@ export function useBaseQuery<TQueryFnData, TError, TData, TQueryData>(
 
   // Handle suspense
   if (observer.options.suspense || observer.options.useErrorBoundary) {
-    if (
-      currentResult.isError &&
-      !errorResetBoundary.isReset() &&
-      !observer.getCurrentQuery().isFetching()
-    ) {
-      throw currentResult.error
-    }
-
     if (observer.options.suspense && currentResult.isLoading) {
       errorResetBoundary.clearReset()
       const unsubscribe = observer.subscribe()
       throw observer.refetch().finally(unsubscribe)
     }
+
+    if (currentResult.isError) {
+      throw currentResult.error
+    }
   }
 
   return currentResult


Test patch:
diff --git a/src/react/tests/QueryResetErrorBoundary.test.tsx b/src/react/tests/QueryResetErrorBoundary.test.tsx
index 79069be70..af0c368ed 100644
--- a/src/react/tests/QueryResetErrorBoundary.test.tsx
+++ b/src/react/tests/QueryResetErrorBoundary.test.tsx
@@ -130,4 +130,71 @@ describe('QueryErrorResetBoundary', () => {
 
     consoleMock.mockRestore()
   })
+
+  it('should never render the component while the query is in error state', async () => {
+    const key = queryKey()
+    const consoleMock = mockConsoleError()
+    let fetchCount = 0
+    let renders = 0
+
+    function Page() {
+      const { data } = useQuery(
+        key,
+        async () => {
+          fetchCount++
+          await sleep(10)
+          if (fetchCount > 2) {
+            return 'data'
+          } else {
+            throw new Error('Error')
+          }
+        },
+        {
+          retry: false,
+          suspense: true,
+        }
+      )
+      renders++
+      return <div>{data}</div>
+    }
+
+    const rendered = renderWithClient(
+      queryClient,
+      <QueryErrorResetBoundary>
+        {({ reset }) => (
+          <ErrorBoundary
+            onReset={reset}
+            fallbackRender={({ resetErrorBoundary }) => (
+              <div>
+                <div>error boundary</div>
+                <button
+                  onClick={() => {
+                    resetErrorBoundary()
+                  }}
+                >
+                  retry
+                </button>
+              </div>
+            )}
+          >
+            <React.Suspense fallback={<div>loading</div>}>
+              <Page />
+            </React.Suspense>
+          </ErrorBoundary>
+        )}
+      </QueryErrorResetBoundary>
+    )
+
+    await waitFor(() => rendered.getByText('error boundary'))
+    await waitFor(() => rendered.getByText('retry'))
+    fireEvent.click(rendered.getByText('retry'))
+    await waitFor(() => rendered.getByText('error boundary'))
+    await waitFor(() => rendered.getByText('retry'))
+    fireEvent.click(rendered.getByText('retry'))
+    await waitFor(() => rendered.getByText('data'))
+    expect(fetchCount).toBe(3)
+    expect(renders).toBe(1)
+
+    consoleMock.mockRestore()
+  })
 })