packages/solid-query/src/__tests__/useQuery.test.tsx (3)

3203-3205: Ensure visibility state is restored even on test failure (wrap in try/finally)

If an assertion fails, the mocked visibility may bleed into subsequent tests. Wrap the body in try/finally to guarantee cleanup.

- // make page unfocused
- const visibilityMock = mockVisibilityState('hidden')
+ // make page unfocused
+ const visibilityMock = mockVisibilityState('hidden')
+ try {
@@
- visibilityMock.mockRestore()
+ } finally {
+ visibilityMock.mockRestore()
+ }

Also applies to: 3255-3256

---

3209-3216: Avoid unnecessary generic on Promise.reject

Promise.reject<unknown>(...) doesn’t add value here and can confuse readers regarding the error type. Returning a rejected promise without the generic keeps intent clear while preserving the string error typing set for useQuery.

- queryFn: () => {
+ queryFn: () => {
 count++
- return Promise.reject<unknown>(`fetching error ${count}`)
+ return Promise.reject(`fetching error ${count}`)
 },

---

3237-3251: Optional: consolidate waits to reduce flakiness and execution time

You perform multiple sequential waitFor checks for related end-state assertions. Consolidating into one waitFor that verifies all required end-state texts reduces the number of polling cycles and makes the test slightly faster and less chatty.

- await vi.waitFor(() =>
- expect(rendered.getByText('failureCount 4')).toBeInTheDocument(),
- )
- await vi.waitFor(() =>
- expect(
- rendered.getByText('failureReason fetching error 4'),
- ).toBeInTheDocument(),
- )
- await vi.waitFor(() =>
- expect(rendered.getByText('status error')).toBeInTheDocument(),
- )
- await vi.waitFor(() =>
- expect(rendered.getByText('error fetching error 4')).toBeInTheDocument(),
- )
+ await vi.waitFor(() => {
+ expect(rendered.getByText('failureCount 4')).toBeInTheDocument()
+ expect(rendered.getByText('failureReason fetching error 4')).toBeInTheDocument()
+ expect(rendered.getByText('status error')).toBeInTheDocument()
+ expect(rendered.getByText('error fetching error 4')).toBeInTheDocument()
+ })

packages/react-query/src/__tests__/useQuery.test.tsx (1)

3434-3479: Guard visibility mock with try/finally to prevent cross-test leakage

If an assertion fails, visibilityMock.mockRestore() won't run and could affect subsequent tests. Wrap the test body in a try/finally.

 it('should continue retry in background even when page is not focused', async () => {
 const key = queryKey()
 // make page unfocused
 const visibilityMock = mockVisibilityState('hidden')
+ try {
 let count = 0
 function Page() {
 const query = useQuery<unknown, string>({
 queryKey: key,
 queryFn: () => {
 count++
 return Promise.reject<unknown>(`fetching error ${count}`)
 },
 retry: 3,
 retryDelay: 1,
 })
 return (
 <div>
 <div>error {String(query.error)}</div>
 <div>status {query.status}</div>
 <div>failureCount {query.failureCount}</div>
 <div>failureReason {query.failureReason}</div>
 </div>
 )
 }
 const rendered = renderWithClient(queryClient, <Page />)
 // With the new behavior, retries continue in background
 // Wait for all retries to complete
 await vi.advanceTimersByTimeAsync(4)
 expect(rendered.getByText('failureCount 4')).toBeInTheDocument()
 expect(
 rendered.getByText('failureReason fetching error 4'),
 ).toBeInTheDocument()
 expect(rendered.getByText('status error')).toBeInTheDocument()
 expect(rendered.getByText('error fetching error 4')).toBeInTheDocument()
 // Verify all retries were attempted
 expect(count).toBe(4)
-
- visibilityMock.mockRestore()
+ } finally {
+ visibilityMock.mockRestore()
+ }
 })

packages/query-core/src/__tests__/query.test.tsx (4)

61-99: Solid background-retry assertion; add try/finally around visibility mock

The updated expectation aligns with removing focus gating. Minor robustness: ensure the visibility mock is restored even on failure.

- const visibilityMock = mockVisibilityState('hidden')
+ const visibilityMock = mockVisibilityState('hidden')
+ try {
 let count = 0
 let result
 const promise = queryClient.fetchQuery({
 queryKey: key,
 queryFn: () => {
 count++
 
 if (count === 3) {
 return `data${count}`
 }
 
 throw new Error(`error${count}`)
 },
 retry: 3,
 retryDelay: 1,
 })
 
 promise.then((data) => {
 result = data
 })
 
 // Check if we do not have a result initially
 expect(result).toBeUndefined()
 
 // With new behavior, retries continue in background
 // Wait for retries to complete
 await vi.advanceTimersByTimeAsync(10)
 expect(result).toBe('data3')
 expect(count).toBe(3)
-
- visibilityMock.mockRestore()
+ } finally {
+ visibilityMock.mockRestore()
+ }

---

149-178: New background-retry test looks good; ensure visibility cleanup via try/finally

Matches the core change to remove focus gating. Add try/finally so the visibility mock is always restored.

- const visibilityMock = mockVisibilityState('hidden')
+ const visibilityMock = mockVisibilityState('hidden')
+ try {
 let count = 0
 let result
 const promise = queryClient.fetchQuery({
 queryKey: key,
 queryFn: () => {
 count++
 if (count === 3) {
 return `data${count}`
 }
 throw new Error(`error${count}`)
 },
 retry: 3,
 retryDelay: 1,
 })
 promise.then((data) => {
 result = data
 })
 // Check if we do not have a result initially
 expect(result).toBeUndefined()
 // Unlike the old behavior, retry should continue in background
 // Wait for retries to complete
 await vi.advanceTimersByTimeAsync(10)
 expect(result).toBe('data3')
 expect(count).toBe(3)
- visibilityMock.mockRestore()
+ } finally {
+ visibilityMock.mockRestore()
+ }

---

180-217: Simplify cancellation assertion and remove redundant checks

You can assert the cancellation more directly and drop the extra result plumbing and redundant instanceof Error check.

- it('should throw a CancelledError when a retrying query is cancelled', async () => {
+ it('should throw a CancelledError when a retrying query is cancelled', async () => {
 const key = queryKey()
 let count = 0
- let result: unknown
 
 const promise = queryClient.fetchQuery({
 queryKey: key,
 queryFn: (): Promise<unknown> => {
 count++
 throw new Error(`error${count}`)
 },
 retry: 3,
- retryDelay: 100, // Longer delay to allow cancellation
+ retryDelay: 100, // Longer delay to allow cancellation
 })
 
- promise.catch((data) => {
- result = data
- })
-
 const query = queryCache.find({ queryKey: key })!
 
 // Wait briefly for first failure and start of retry
 await vi.advanceTimersByTimeAsync(1)
 expect(result).toBeUndefined()
 
 // Cancel query during retry
 query.cancel()
 
- // Check if the error is set to the cancelled error
- try {
- await promise
- expect.unreachable()
- } catch {
- expect(result instanceof CancelledError).toBe(true)
- expect(result instanceof Error).toBe(true)
- }
+ // Assert promise rejects with CancelledError
+ await expect(promise).rejects.toBeInstanceOf(CancelledError)
 })

Note: if you keep the current style, consider adding await vi.advanceTimersByTimeAsync(0) after query.cancel() to flush microtasks, but it isn’t strictly necessary when awaiting the promise.

---

258-294: Good coverage of refetchInterval + retries in background; add subscription cleanup and try/finally

• Capture and call the unsubscribe returned from subscribe.
• Wrap with try/finally to always destroy the observer and restore visibility, even if an assertion fails.

 it('should continue refetchInterval with retries in background when tab is inactive', async () => {
 const key = queryKey()
- const visibilityMock = mockVisibilityState('hidden')
+ const visibilityMock = mockVisibilityState('hidden')
 
 let totalRequests = 0
 
 const queryObserver = new QueryObserver(queryClient, {
 queryKey: key,
 queryFn: () => {
 totalRequests++
 // Always fail to simulate network offline
 throw new Error(`Network error ${totalRequests}`)
 },
 refetchInterval: 60000,
 refetchIntervalInBackground: true,
 retry: 3,
 retryDelay: 1,
 })
 
- queryObserver.subscribe(() => {})
+ const unsubscribe = queryObserver.subscribe(() => {})
 
- // First interval: t=0 to t=60s (initial query + retries)
- await vi.advanceTimersByTimeAsync(60000)
- expect(totalRequests).toBe(4)
-
- // Second interval: t=60s to t=120s (refetch + retries)
- await vi.advanceTimersByTimeAsync(60000)
- expect(totalRequests).toBe(8)
-
- // Third interval: t=120s to t=180s (refetch + retries)
- await vi.advanceTimersByTimeAsync(60000)
- expect(totalRequests).toBe(12)
-
- queryObserver.destroy()
- visibilityMock.mockRestore()
+ try {
+ // First interval: t=0 to t=60s (initial query + retries)
+ await vi.advanceTimersByTimeAsync(60000)
+ expect(totalRequests).toBe(4)
+
+ // Second interval: t=60s to t=120s (refetch + retries)
+ await vi.advanceTimersByTimeAsync(60000)
+ expect(totalRequests).toBe(8)
+
+ // Third interval: t=120s to t=180s (refetch + retries)
+ await vi.advanceTimersByTimeAsync(60000)
+ expect(totalRequests).toBe(12)
+ } finally {
+ unsubscribe()
+ queryObserver.destroy()
+ visibilityMock.mockRestore()
+ }
 })

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

packages/query-core/src/__tests__/query.test.tsx (4)

89-97: Avoid magic timer values; flush timers deterministically

Using a fixed 10ms advance is brittle. Prefer running pending timers to completion in this test to avoid coupling to retryDelay:

-// With new behavior, retries continue in background
-// Wait for retries to complete
-await vi.advanceTimersByTimeAsync(10)
-expect(result).toBe('data3')
+// Flush scheduled retries deterministically
+await vi.runAllTimersAsync()
+expect(result).toBe('data3')

If you want to keep time-based advancing, compute from the configured retryDelay and number of retries to document intent (e.g., advance by retryDelay * 3 + epsilon).

---

149-178: Duplicate of the previous test; consider consolidating

This test appears to assert the same behavior as the earlier "continue retry ... while hidden" case with identical setup and expectations. Consider merging into a single, descriptively named test or parameterizing to reduce duplication and test time.

Example consolidation (rename earlier test and remove this one), or parameterize with it.each over an array of titles if you need both names for historical clarity.

---

180-217: Tighten cancellation assertions and timing

• Use toBeInstanceOf for clarity/readability:

- expect(result instanceof CancelledError).toBe(true)
- expect(result instanceof Error).toBe(true)
+ expect(result).toBeInstanceOf(CancelledError)

• To make "cancel during retry" explicit with retryDelay: 100, consider advancing to a point between attempts (e.g., 50ms) before cancelling:

-// Wait briefly for first failure and start of retry
-await vi.advanceTimersByTimeAsync(1)
+// Wait into the backoff window before the next retry
+await vi.advanceTimersByTimeAsync(50)

This reduces ambiguity between "after initial failure but before retry starts" vs "during a retry backoff window".

---

258-293: Good coverage of refetchIntervalInBackground; minor cleanup suggested

• The test accurately validates background refetch+retry behavior when hidden with refetchIntervalInBackground: true. Nice.
• Minor test hygiene: subscribe returns an unsubscribe function—prefer using it to clean up instead of destroy(), which is more forceful than needed here:

- queryObserver.subscribe(() => {})
+ const unsubscribe = queryObserver.subscribe(() => {})
 ...
- queryObserver.destroy()
+ unsubscribe()

• Optional: make interval values self-documenting and avoid repeated literals:

- refetchInterval: 60000,
+ const interval = 60_000
+ ...
+ refetchInterval: interval,
...
- await vi.advanceTimersByTimeAsync(60000)
+ await vi.advanceTimersByTimeAsync(interval)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

packages/query-core/src/types.ts (2)

279-281: TSDoc: clarify that this also affects retry gating, not just interval refetching

With this PR, the flag now also allows retries to continue while unfocused. Consider clarifying the description and a tiny grammar tweak.

Apply this diff to update the comment:

 /**
- * If set to `true`, the query will continue to refetch while their tab/window is in the background.
- * Defaults to `false`.
+ * If set to `true`, the query will continue to refetch and retry even when the tab/window is in the background.
+ * Defaults to `false`.
 */

---

278-283: Avoid duplicate declarations between QueryOptions and QueryObserverOptions

Now that QueryObserverOptions extends QueryOptions and this property lives on QueryOptions, the explicit redeclaration on QueryObserverOptions (Line 354–357) is redundant and risks docs drift.

Two options:

• Remove the property from QueryObserverOptions and let it inherit from QueryOptions (preferred).
• If you want to keep docs colocated on the observer interface, remove it from QueryOptions and fetch it from the first observer’s options when wiring the retryer (heavier change).

If you opt for the first approach, remove the member from QueryObserverOptions (outside this hunk).

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

packages/query-core/src/types.ts (2)

278-283: Clarify that this flag governs retries as well, and avoid duplicate declarations drifting over time

Good addition. Since retryer.ts now uses this option to allow background retries, the JSDoc should reflect that it affects both retries and interval refetches. Also, this property is declared here and again in QueryObserverOptions (Lines 354-357) with slightly different wording; consider consolidating to a single source to prevent future drift.

Apply this doc tweak within the current hunk:

 /**
- * If set to `true`, the query will continue to refetch while their tab/window is in the background.
- * Defaults to `false`.
+ * If set to `true`, the query will continue retries and refetches while the tab/window is in the background.
+ * Also influences retry behavior: background retries will proceed even when unfocused. Defaults to `false`.
 */
 refetchIntervalInBackground?: boolean

Optional consolidation (follow-up): Remove the duplicate property from QueryObserverOptions and rely on inheritance, or keep both but make their JSDoc identical. If your docs rely on the sub-interface re-declaration, keep it and sync the wording (see next comment).

---

354-357: Keep wording consistent here to match the new behavior

Mirror the updated semantics so both declarations say the same thing.

Suggested text:

/**
 * If set to `true`, the query will continue retries and refetches while the tab/window is in the background.
 * Defaults to `false`.
 */
refetchIntervalInBackground?: boolean

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.