Skip to content
Open
186 changes: 186 additions & 0 deletions packages/react-query/src/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5920,6 +5920,7 @@ describe('useQuery', () => {
it('should be able to toggle subscribed', async () => {
const key = queryKey()
const queryFn = vi.fn(() => Promise.resolve('data'))

function Page() {
const [subscribed, setSubscribed] = React.useState(true)
const { data } = useQuery({
Expand Down Expand Up @@ -5964,6 +5965,7 @@ describe('useQuery', () => {
it('should not be attached to the query when subscribed is false', async () => {
const key = queryKey()
const queryFn = vi.fn(() => Promise.resolve('data'))

function Page() {
const { data } = useQuery({
queryKey: key,
Expand Down Expand Up @@ -5992,6 +5994,7 @@ describe('useQuery', () => {
it('should not re-render when data is added to the cache when subscribed is false', async () => {
const key = queryKey()
let renders = 0

function Page() {
const { data } = useQuery({
queryKey: key,
Expand Down Expand Up @@ -6191,6 +6194,7 @@ describe('useQuery', () => {
await sleep(5)
return { numbers: { current: { id } } }
}

function Test() {
const [id, setId] = React.useState(1)

Expand Down Expand Up @@ -6256,6 +6260,7 @@ describe('useQuery', () => {
await sleep(5)
return { numbers: { current: { id } } }
}

function Test() {
const [id, setId] = React.useState(1)

Expand Down Expand Up @@ -6761,10 +6766,12 @@ describe('useQuery', () => {
it('should console.error when there is no queryFn', () => {
const consoleErrorMock = vi.spyOn(console, 'error')
const key = queryKey()

function Example() {
useQuery({ queryKey: key })
return <></>
}

renderWithClient(queryClient, <Example />)

expect(consoleErrorMock).toHaveBeenCalledTimes(1)
Expand All @@ -6774,4 +6781,183 @@ describe('useQuery', () => {

consoleErrorMock.mockRestore()
})

it('should retry on mount when throwOnError returns false', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})

function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: () => false,
retryOnMount: true,
staleTime: Infinity,
retry: false,
})

return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}

const { unmount, getByTestId } = renderWithClient(
queryClient,
<Component />,
)

await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)

unmount()

const initialFetchCount = fetchCount

renderWithClient(queryClient, <Component />)

await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)

expect(fetchCount).toBe(initialFetchCount + 1)
expect(queryFn).toHaveBeenCalledTimes(2)
})
Comment on lines +6785 to +6835
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix test flakiness: use fake-timer flush and don’t reuse getByTestId after unmount.

  • vi.waitFor is not a vitest API; flush with vi.advanceTimersByTimeAsync(0) instead (fake timers are enabled).
  • Don’t reuse queries from an unmounted render; capture a new render result.

Apply this diff:

-    const { unmount, getByTestId } = renderWithClient(
-      queryClient,
-      <Component />,
-    )
-
-    await vi.waitFor(() =>
-      expect(getByTestId('status')).toHaveTextContent('error'),
-    )
-    expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
+    const rendered1 = renderWithClient(queryClient, <Component />)
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered1.getByTestId('status')).toHaveTextContent('error')
+    expect(rendered1.getByTestId('error')).toHaveTextContent(
+      'Simulated 500 error',
+    )
-    expect(fetchCount).toBe(1)
-
-    unmount()
+    expect(fetchCount).toBe(1)
+    rendered1.unmount()
@@
-    renderWithClient(queryClient, <Component />)
-
-    await vi.waitFor(() =>
-      expect(getByTestId('status')).toHaveTextContent('error'),
-    )
+    const rendered2 = renderWithClient(queryClient, <Component />)
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered2.getByTestId('status')).toHaveTextContent('error')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('should retry on mount when throwOnError returns false', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})
function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: () => false,
retryOnMount: true,
staleTime: Infinity,
retry: false,
})
return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}
const { unmount, getByTestId } = renderWithClient(
queryClient,
<Component />,
)
await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)
unmount()
const initialFetchCount = fetchCount
renderWithClient(queryClient, <Component />)
await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
expect(fetchCount).toBe(initialFetchCount + 1)
expect(queryFn).toHaveBeenCalledTimes(2)
})
it('should retry on mount when throwOnError returns false', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})
function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: () => false,
retryOnMount: true,
staleTime: Infinity,
retry: false,
})
return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}
// First mount
const rendered1 = renderWithClient(queryClient, <Component />)
await vi.advanceTimersByTimeAsync(0)
expect(rendered1.getByTestId('status')).toHaveTextContent('error')
expect(
rendered1.getByTestId('error')
).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)
rendered1.unmount()
const initialFetchCount = fetchCount
// Second mount
const rendered2 = renderWithClient(queryClient, <Component />)
await vi.advanceTimersByTimeAsync(0)
expect(rendered2.getByTestId('status')).toHaveTextContent('error')
expect(fetchCount).toBe(initialFetchCount + 1)
expect(queryFn).toHaveBeenCalledTimes(2)
})
🤖 Prompt for AI Agents
packages/react-query/src/__tests__/useQuery.test.tsx around lines 6785 to 6835:
the test is flaky because it uses vi.waitFor (not a vitest API with fake timers)
and reuses getByTestId from a component after unmount; replace the vi.waitFor
flush with vi.advanceTimersByTimeAsync(0) to advance fake timers and ensure task
queue is flushed, and when re-rendering after unmount capture the new render
result (assign the return of renderWithClient to a new variable) and call
getByTestId on that new result instead of reusing the previous one so assertions
target a mounted DOM.


it('should not retry on mount when throwOnError function returns true', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})

function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: () => true,
retryOnMount: true,
staleTime: Infinity,
retry: false,
})

return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}

const { unmount, getByTestId } = renderWithClient(
queryClient,
<ErrorBoundary
fallbackRender={({ error }) => (
<div>
<div data-testid="status">error</div>
<div data-testid="error">{error?.message}</div>
</div>
)}
>
<Component />
</ErrorBoundary>,
)

await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)

unmount()

const initialFetchCount = fetchCount

renderWithClient(
queryClient,
<ErrorBoundary
fallbackRender={({ error }) => (
<div>
<div data-testid="status">error</div>
<div data-testid="error">{error?.message}</div>
</div>
)}
>
<Component />
</ErrorBoundary>,
)

await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)

// Should not retry because throwOnError returns true
expect(fetchCount).toBe(initialFetchCount)
expect(queryFn).toHaveBeenCalledTimes(1)
})
Comment on lines +6837 to +6909
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Same test issues: replace vi.waitFor and avoid stale container after unmount.

Mirror the fixes from the previous test; also keep assertions via the newly captured render result.

-    const { unmount, getByTestId } = renderWithClient(
-      queryClient,
-      <ErrorBoundary
-        fallbackRender={({ error }) => (
-          <div>
-            <div data-testid="status">error</div>
-            <div data-testid="error">{error?.message}</div>
-          </div>
-        )}
-      >
-        <Component />
-      </ErrorBoundary>,
-    )
-
-    await vi.waitFor(() =>
-      expect(getByTestId('status')).toHaveTextContent('error'),
-    )
-    expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
+    const rendered1 = renderWithClient(
+      queryClient,
+      <ErrorBoundary
+        fallbackRender={({ error }) => (
+          <div>
+            <div data-testid="status">error</div>
+            <div data-testid="error">{error?.message}</div>
+          </div>
+        )}
+      >
+        <Component />
+      </ErrorBoundary>,
+    )
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered1.getByTestId('status')).toHaveTextContent('error')
+    expect(rendered1.getByTestId('error')).toHaveTextContent('Simulated 500 error')
@@
-    unmount()
+    rendered1.unmount()
@@
-    renderWithClient(
-      queryClient,
-      <ErrorBoundary
-        fallbackRender={({ error }) => (
-          <div>
-            <div data-testid="status">error</div>
-            <div data-testid="error">{error?.message}</div>
-          </div>
-        )}
-      >
-        <Component />
-      </ErrorBoundary>,
-    )
-
-    await vi.waitFor(() =>
-      expect(getByTestId('status')).toHaveTextContent('error'),
-    )
+    const rendered2 = renderWithClient(
+      queryClient,
+      <ErrorBoundary
+        fallbackRender={({ error }) => (
+          <div>
+            <div data-testid="status">error</div>
+            <div data-testid="error">{error?.message}</div>
+          </div>
+        )}
+      >
+        <Component />
+      </ErrorBoundary>,
+    )
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered2.getByTestId('status')).toHaveTextContent('error')
🤖 Prompt for AI Agents
In packages/react-query/src/__tests__/useQuery.test.tsx around lines 6837 to
6909, the test still uses vi.waitFor and reads from a stale container after
unmount; replace vi.waitFor with the testing-library async waitFor helper and
capture the render result for the second render to avoid querying the old
container. Specifically, import/use waitFor (not vi.waitFor), store the result
of the second renderWithClient call in a variable (e.g., const { getByTestId } =
renderWithClient(...)) and use that getByTestId for the post-unmount assertions,
and update the async waits to await waitFor(() => expect(...)) so the test no
longer queries a stale container.


it('should handle throwOnError function based on actual error state', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})

function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: (error) => error.message.includes('404'),
retryOnMount: true,
staleTime: Infinity,
retry: false,
})

return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}

const { unmount, getByTestId } = renderWithClient(
queryClient,
<Component />,
)

await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)

unmount()

const initialFetchCount = fetchCount

renderWithClient(queryClient, <Component />)

await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)

// Should retry because throwOnError returns false (500 error doesn't include '404')
expect(fetchCount).toBe(initialFetchCount + 1)
expect(queryFn).toHaveBeenCalledTimes(2)
})
Comment on lines +6911 to +6962
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Same fixes for the 404/500 predicate case.

Use timer flush and distinct render containers after remount.

-    const { unmount, getByTestId } = renderWithClient(
-      queryClient,
-      <Component />,
-    )
-
-    await vi.waitFor(() =>
-      expect(getByTestId('status')).toHaveTextContent('error'),
-    )
-    expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
+    const rendered1 = renderWithClient(queryClient, <Component />)
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered1.getByTestId('status')).toHaveTextContent('error')
+    expect(rendered1.getByTestId('error')).toHaveTextContent('Simulated 500 error')
@@
-    unmount()
+    rendered1.unmount()
@@
-    renderWithClient(queryClient, <Component />)
-
-    await vi.waitFor(() =>
-      expect(getByTestId('status')).toHaveTextContent('error'),
-    )
+    const rendered2 = renderWithClient(queryClient, <Component />)
+    await vi.advanceTimersByTimeAsync(0)
+    expect(rendered2.getByTestId('status')).toHaveTextContent('error')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('should handle throwOnError function based on actual error state', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})
function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: (error) => error.message.includes('404'),
retryOnMount: true,
staleTime: Infinity,
retry: false,
})
return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}
const { unmount, getByTestId } = renderWithClient(
queryClient,
<Component />,
)
await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
expect(getByTestId('error')).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)
unmount()
const initialFetchCount = fetchCount
renderWithClient(queryClient, <Component />)
await vi.waitFor(() =>
expect(getByTestId('status')).toHaveTextContent('error'),
)
// Should retry because throwOnError returns false (500 error doesn't include '404')
expect(fetchCount).toBe(initialFetchCount + 1)
expect(queryFn).toHaveBeenCalledTimes(2)
})
it('should handle throwOnError function based on actual error state', async () => {
const key = queryKey()
let fetchCount = 0
const queryFn = vi.fn().mockImplementation(() => {
fetchCount++
console.log(`Fetching... (attempt ${fetchCount})`)
return Promise.reject(new Error('Simulated 500 error'))
})
function Component() {
const { status, error } = useQuery({
queryKey: key,
queryFn,
throwOnError: (error) => error.message.includes('404'),
retryOnMount: true,
staleTime: Infinity,
retry: false,
})
return (
<div>
<div data-testid="status">{status}</div>
{error && <div data-testid="error">{error.message}</div>}
</div>
)
}
// First render
const rendered1 = renderWithClient(queryClient, <Component />)
await vi.advanceTimersByTimeAsync(0)
expect(rendered1.getByTestId('status')).toHaveTextContent('error')
expect(rendered1.getByTestId('error')).toHaveTextContent('Simulated 500 error')
expect(fetchCount).toBe(1)
rendered1.unmount()
const initialFetchCount = fetchCount
// Remount in a fresh container
const rendered2 = renderWithClient(queryClient, <Component />)
await vi.advanceTimersByTimeAsync(0)
expect(rendered2.getByTestId('status')).toHaveTextContent('error')
// Should retry because throwOnError returns false (500 error doesn't include '404')
expect(fetchCount).toBe(initialFetchCount + 1)
expect(queryFn).toHaveBeenCalledTimes(2)
})

})
18 changes: 13 additions & 5 deletions packages/react-query/src/errorBoundaryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,24 @@ export const ensurePreventErrorBoundaryRetry = <
TQueryKey
>,
errorResetBoundary: QueryErrorResetBoundaryValue,
query?: Query<TQueryFnData, TError, TQueryData, TQueryKey>,
) => {
if (
options.suspense ||
options.throwOnError ||
options.experimental_prefetchInRender
) {
if (options.suspense || options.experimental_prefetchInRender) {
// Prevent retrying failed query if the error boundary has not been reset yet
if (!errorResetBoundary.isReset()) {
options.retryOnMount = false
}
} else if (options.throwOnError && !errorResetBoundary.isReset()) {
if (typeof options.throwOnError === 'function') {
if (
query?.state.error &&
shouldThrowError(options.throwOnError, [query.state.error, query])
) {
options.retryOnMount = false
}
} else {
options.retryOnMount = false
}
}
}

Expand Down
11 changes: 9 additions & 2 deletions packages/react-query/src/useBaseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ export function useBaseQuery<
const errorResetBoundary = useQueryErrorResetBoundary()
const client = useQueryClient(queryClient)
const defaultedOptions = client.defaultQueryOptions(options)
const query = client
.getQueryCache()
.get<
TQueryFnData,
TError,
TQueryData,
TQueryKey
>(defaultedOptions.queryHash)
Comment on lines +56 to +63
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Compute the Query after _experimental_beforeQuery to avoid stale queryHash mismatches.

You fetch the cached query before calling _experimental_beforeQuery, which can mutate options (incl. queryKey/queryHash). This risks passing the wrong query into ensurePreventErrorBoundaryRetry. Inline the cache lookup at the call-site after the hook and drop this precomputed variable.

Apply this diff:

-  const query = client
-    .getQueryCache()
-    .get<
-      TQueryFnData,
-      TError,
-      TQueryData,
-      TQueryKey
-    >(defaultedOptions.queryHash)

Committable suggestion skipped: line range outside the PR's diff.


;(client.getDefaultOptions().queries as any)?._experimental_beforeQuery?.(
defaultedOptions,
Expand All @@ -72,8 +80,7 @@ export function useBaseQuery<
: 'optimistic'

ensureSuspenseTimers(defaultedOptions)
ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary)

ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query)
useClearResetErrorBoundary(errorResetBoundary)
Comment on lines 82 to 84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use a fresh cache lookup when calling ensurePreventErrorBoundaryRetry.

Inline the query read here so it reflects any changes done by _experimental_beforeQuery.

-  ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query)
+  ensurePreventErrorBoundaryRetry(
+    defaultedOptions,
+    errorResetBoundary,
+    client
+      .getQueryCache()
+      .get<TQueryFnData, TError, TQueryData, TQueryKey>(
+        defaultedOptions.queryHash,
+      ),
+  )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ensureSuspenseTimers(defaultedOptions)
ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary)
ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary, query)
useClearResetErrorBoundary(errorResetBoundary)
ensureSuspenseTimers(defaultedOptions)
ensurePreventErrorBoundaryRetry(
defaultedOptions,
errorResetBoundary,
client
.getQueryCache()
.get<TQueryFnData, TError, TQueryData, TQueryKey>(
defaultedOptions.queryHash,
),
)
useClearResetErrorBoundary(errorResetBoundary)
🤖 Prompt for AI Agents
In packages/react-query/src/useBaseQuery.ts around lines 82 to 84, inline a
fresh query cache lookup when calling ensurePreventErrorBoundaryRetry instead of
passing the stale "query" variable so changes from _experimental_beforeQuery are
reflected; replace the current call with one that reads the up-to-date query
from the queryCache (e.g. perform a direct cache lookup/find here) and pass that
freshQuery into ensurePreventErrorBoundaryRetry, keeping the surrounding
ensureSuspenseTimers and useClearResetErrorBoundary calls unchanged.


// this needs to be invoked before creating the Observer because that can create a cache entry
Expand Down
7 changes: 4 additions & 3 deletions packages/react-query/src/useQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,10 @@ export function useQueries<
[queries, client, isRestoring],
)

defaultedQueries.forEach((query) => {
ensureSuspenseTimers(query)
ensurePreventErrorBoundaryRetry(query, errorResetBoundary)
defaultedQueries.forEach((queryOptions) => {
ensureSuspenseTimers(queryOptions)
const query = client.getQueryCache().get(queryOptions.queryHash)
ensurePreventErrorBoundaryRetry(queryOptions, errorResetBoundary, query)
})

useClearResetErrorBoundary(errorResetBoundary)
Expand Down