feat(query-core): add timeoutManager to allow changing setTimeout/setInterval #9612
Conversation
WalkthroughAdds a pluggable TimeoutManager and systemSetTimeoutZero, migrates internal timers to use it or notifyManager, expands query-core public exports, adds timeoutManager tests, updates persisters/throttles to managed timers, and updates documentation and config entries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Consumer as Component / Observer
participant Manager as timeoutManager
participant Provider as TimeoutProvider
participant System as System Timers
Consumer->>Manager: setTimeout(callback, delay)
Manager->>Provider: provider.setTimeout(callback, delay)
Provider-->>Manager: providerTimerId (ManagedTimerId)
Manager-->>Consumer: ManagedTimerId
Consumer->>Manager: clearTimeout(ManagedTimerId)
Manager->>Provider: provider.clearTimeout(ManagedTimerId)
sequenceDiagram
autonumber
participant Caller as notifyManager/defaultScheduler
participant Z as systemSetTimeoutZero
participant System as System Timers
Caller->>Z: schedule callback with 0ms
Z->>System: setTimeout(callback, 0)
Note over Z,System: Bypasses timeoutManager to use platform 0ms scheduling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/query-core/src/timeoutManager.ts (2)
2-5: Prefer callback type () => void for broad compatibility.Typing as
(_: void) => voidcan be awkward for consumers with zero-arg callbacks and is inconsistent with internal types (e.g., notifyManager). Using() => voidcleanly communicates “no args are supported.”Apply this diff:
-export type TimeoutCallback = (_: void) => void +export type TimeoutCallback = () => void
71-86: Provider switching after first use is a footgun; consider freezing or tagging IDs.Warning is helpful, but accidental provider swaps will silently leak timers. Either:
- Disallow switching after first use (throw), or
- Tag IDs with the provider (e.g., wrap
{ provider, id }) so clear* always hits the correct backend.Example (freeze after first use):
if (this.#providerCalled) { - console.warn( - `[timeoutManager]: Switching to ${provider.name} provider after calls to ${this.#provider.name} provider might result in unexpected behavior.`, - ) + throw new Error( + `[timeoutManager]: Provider already in use (${this.#provider.name}); set a provider before creating timers.`, + ) }If you prefer switching, I can sketch a lightweight branded handle that carries the provider without a registry.
packages/query-core/src/notifyManager.ts (1)
15-16: Nit: assign the function directly.Saves an extra wrapper and preserves the function identity.
-export const defaultScheduler: ScheduleFunction = (cb) => - systemSetTimeoutZero(cb) +export const defaultScheduler: ScheduleFunction = systemSetTimeoutZeropackages/query-core/src/utils.ts (1)
365-367: Nit: simplify call-site to avoid the temporary variable.Clearer and avoids a union-typed variable.
- const setTimeoutFn = - timeout === 0 ? systemSetTimeoutZero : managedSetTimeout - setTimeoutFn(resolve, timeout) + if (timeout === 0) { + systemSetTimeoutZero(resolve) + } else { + managedSetTimeout(resolve, timeout) + }Optional: consider
sleep(timeout, signal?: AbortSignal)to allow cancellation; I can provide an implementation if useful.packages/query-async-storage-persister/src/asyncThrottle.ts (2)
25-25: Avoid accidental zero-delay semanticsClamp to at least 1ms so this always routes through managedSetTimeout’s >0 path.
- await new Promise((done) => managedSetTimeout(done, interval)) + await new Promise((done) => managedSetTimeout(done, Math.max(1, interval)))
29-30: Race-proof the computed delayBetween the while-check and the call, the delta can hit 0. Clamp to >=1ms.
- managedSetTimeout(done, nextExecutionTime - Date.now()), + managedSetTimeout(done, Math.max(1, nextExecutionTime - Date.now())),packages/query-core/src/index.ts (2)
14-22: Consider exporting the manager and provider type for parity.If you want full parity with
focusManager/onlineManagerpatterns and to enable custom providers, consider also exportingtimeoutManager,TimeoutManager, and the provider type.Proposed diff:
export { managedSetTimeout, managedClearTimeout, managedSetInterval, managedClearInterval, systemSetTimeoutZero, defaultTimeoutProvider, + timeoutManager, + TimeoutManager, + type TimeoutProvider, type ManagedTimerId, } from './timeoutManager'(Assumes these symbols are exported from
./timeoutManager.)
14-22: Optional: keep baseline bundle lean.If baseline size is a concern, consider not re-exporting these from the root barrel and instead documenting a subpath import (e.g.,
@tanstack/query-core/timeoutManager) so apps only pull this module when needed.
📜 Review details
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.
📒 Files selected for processing (9)
packages/query-async-storage-persister/src/asyncThrottle.ts(2 hunks)packages/query-core/src/index.ts(1 hunks)packages/query-core/src/notifyManager.ts(2 hunks)packages/query-core/src/queryObserver.ts(5 hunks)packages/query-core/src/removable.ts(3 hunks)packages/query-core/src/timeoutManager.ts(1 hunks)packages/query-core/src/utils.ts(2 hunks)packages/query-persist-client-core/src/createPersister.ts(3 hunks)packages/query-sync-storage-persister/src/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/query-core/src/removable.ts (1)
packages/query-core/src/queryObserver.ts (1)
managedClearTimeout(419-424)
packages/query-core/src/utils.ts (1)
packages/query-core/src/index.ts (2)
systemSetTimeoutZero(19-19)managedSetTimeout(15-15)
packages/query-core/src/notifyManager.ts (1)
packages/query-core/src/index.ts (2)
defaultScheduler(13-13)systemSetTimeoutZero(19-19)
packages/query-core/src/timeoutManager.ts (1)
packages/query-core/src/index.ts (7)
defaultTimeoutProvider(20-20)ManagedTimerId(21-21)managedSetTimeout(15-15)managedClearTimeout(16-16)managedSetInterval(17-17)managedClearInterval(18-18)systemSetTimeoutZero(19-19)
🔇 Additional comments (19)
packages/query-core/src/timeoutManager.ts (1)
13-16: Incorrect: conversion to number is safe and supported NodeJS.Timeout objects implementSymbol.toPrimitive(added in v12.19.0/v14.9.0) returning an integer, andclearTimeout/clearIntervalaccept bothTimeoutobjects and their numeric primitives across Node.js v16–22 (nodejs.org). The existingTimeoutProviderIdtype already enforces provider support forSymbol.toPrimitiveor numeric handles—no refactor required.Likely an incorrect or invalid review comment.
packages/query-core/src/notifyManager.ts (1)
3-3: LGTM: default scheduler now uses systemSetTimeoutZero.Consistent with the new timeoutManager API and preserves “next tick” semantics.
Also applies to: 15-16
packages/query-core/src/utils.ts (1)
1-1: LGTM: importing managedSetTimeout/systemSetTimeoutZero aligns sleep with the new manager.packages/query-core/src/removable.ts (3)
1-1: LGTM: timer-manager integration (imports/types).Also applies to: 3-3, 7-7
17-20: LGTM: GC scheduling now uses managedSetTimeout.
33-34: LGTM: proper cleanup via managedClearTimeout.packages/query-core/src/queryObserver.ts (5)
16-23: LGTM: switch to managed timer APIs and types.
72-74: LGTM: ManagedTimerId for internal handles.
375-380: LGTM: stale-time timeout uses managedSetTimeout.
404-412: LGTM: refetch interval via managedSetInterval; guards are intact.
421-422: LGTM: clear paths updated to managed clear functions.Also applies to: 428-429
packages/query-persist-client-core/src/createPersister.ts (3)
1-6: LGTM: import systemSetTimeoutZero from core.
134-134: LGTM: restore callback scheduled on next tick via systemSetTimeoutZero.
221-224: LGTM: persist scheduled on next tick via systemSetTimeoutZero.packages/query-sync-storage-persister/src/index.ts (3)
1-1: LGTM: consuming managed timers and types from core.Also applies to: 3-3
105-105: LGTM: ManagedTimerId for throttle handle.
110-114: LGTM: throttle uses managedSetTimeout.packages/query-core/src/index.ts (2)
14-22: Public API additions look coherent; type-only re-export is good.Re-exporting the managed timer helpers and default provider from the barrel aligns with Focus/Online patterns. Exporting
type ManagedTimerIdas a type-only binding avoids runtime cost. LGTM.
14-22: timeoutManager module import is side-effect free and SSR/React Native safe
No top-level browser globals or immediate timer calls—only an emptyTimeoutManagerinstantiation and provider definitions using global timer APIs—so SSR and React Native imports pose no side effects.
|
View your CI Pipeline Execution ↗ for commit 7922966
☁️ Nx Cloud last updated this comment at |
There was a problem hiding this comment.
bundle size impact looks okay:
before: 44.2kb (13.1kb gzip)
after: 45.5kb (13.6kb gzip)
I’m not sure if the extra functions are worth it, we should try out how this performs without them. Even if they aren’t minifyable on the call-sides, they usually gzip quite well.
| @@ -10,7 +12,8 @@ type BatchCallsCallback<T extends Array<unknown>> = (...args: T) => void | |||
|
|
|||
| type ScheduleFunction = (callback: () => void) => void | |||
|
|
|||
| export const defaultScheduler: ScheduleFunction = (cb) => setTimeout(cb, 0) | |||
| export const defaultScheduler: ScheduleFunction = (cb) => | |||
There was a problem hiding this comment.
note: we want to change this to scheduleMicroTask in the next major version, so maybe we should keep this outside of the managed timeouts to avoid larger changes when someone opts into their own managed timeout
There was a problem hiding this comment.
That's why I have it calling systemSetTimeoutZero - no change in semantics, and the code clearly shows that it's not an oversight
There was a problem hiding this comment.
so you’re saying places where we now use systemSetTimeoutZero we should in the future use scheduleMicroTask ?
There was a problem hiding this comment.
ah, I’m just now realizing that systemSetTimeoutZero doesn’t use the timeoutManager 😅 . I get it now 👍
There was a problem hiding this comment.
so you’re saying places where we now use
systemSetTimeoutZerowe should in the future usescheduleMicroTask?
I can't make that assessment - it's up to you to decide that should remain globalThis.setTimeout(cb, 0) and what should change to your new scheduleMicroTask function.
But yeah, my intention is that changes from setTimeout(cb, 0) to systemSetTimeoutZero are syntactic only (to ease code review going forward), and remain semantically unchanged compared to before this PR.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
packages/query-core/src/__tests__/timeoutManager.test.tsx (8)
39-59: Strengthen default-proxy assertions by verifying cleared handles tooAsserting the exact handle passed to clearTimeout/clearInterval increases confidence the proxying works end-to-end.
const timeoutId = manager.setTimeout(callback, 100) expect(setTimeoutSpy).toHaveBeenCalledWith(callback, 100) clearTimeout(timeoutId) + expect(clearTimeoutSpy).toHaveBeenCalledWith(timeoutId) manager.clearTimeout(200) expect(clearTimeoutSpy).toHaveBeenCalledWith(200) const intervalId = manager.setInterval(callback, 300) expect(setIntervalSpy).toHaveBeenCalledWith(callback, 300) clearInterval(intervalId) + expect(clearIntervalSpy).toHaveBeenCalledWith(intervalId) manager.clearInterval(400) expect(clearIntervalSpy).toHaveBeenCalledWith(400)
61-79: Also assert return values from provider-backed callsValidates that provider IDs propagate through the manager.
- manager.setTimeout(callback, 100) - expect(customProvider.setTimeout).toHaveBeenCalledWith(callback, 100) + const tId = manager.setTimeout(callback, 100) + expect(customProvider.setTimeout).toHaveBeenCalledWith(callback, 100) + expect(tId).toBe(123) @@ - manager.setInterval(callback, 200) - expect(customProvider.setInterval).toHaveBeenCalledWith(callback, 200) + const iId = manager.setInterval(callback, 200) + expect(customProvider.setInterval).toHaveBeenCalledWith(callback, 200) + expect(iId).toBe(456)
81-102: Cover no-op provider re-set (same instance) should not warnEnsures the early-return path in setTimeoutProvider is exercised.
const customProvider3 = createMockProvider('custom3') manager.setTimeoutProvider(customProvider3) expect(console.warn).not.toHaveBeenCalled() + // 4. Setting the same provider again is a no-op (no warn) + vi.mocked(console.warn).mockClear() + manager.setTimeoutProvider(customProvider3) + expect(console.warn).not.toHaveBeenCalled()
114-116: Relax brittle error-message assertionsMatching on a regex avoids coupling tests to exact wording while still verifying the failure mode.
- expect(() => manager.setTimeout(callback, 100)).toThrow( - 'TimeoutManager: could not convert badProvider provider timeout ID to valid number', - ) + expect(() => manager.setTimeout(callback, 100)).toThrowError( + /could not convert .*badProvider.*timeout ID/i, + ) @@ - expect(() => manager.setInterval(callback, 100)).toThrow( - 'TimeoutManager: could not convert badProvider provider timeout ID to valid number', - ) + expect(() => manager.setInterval(callback, 100)).toThrowError( + /could not convert .*badProvider.*timeout ID/i, + )Also applies to: 118-120
169-179: Restore spy for symmetry with other testsKeeps spy lifecycle consistent within the block (outer restoreAllMocks still works, this just localizes cleanup).
expect(spy).toHaveBeenCalledWith(...args) expect(result).toBe(456) + spy.mockRestore()
192-201: Assert call count for systemSetTimeoutZeroMinor strengthening to ensure exactly one scheduling call happened.
expect(spy).toHaveBeenCalledWith(callback, 0) + expect(spy).toHaveBeenCalledTimes(1) clearTimeout(spy.mock.results[0]?.value)
130-141: Add a no-op undefined clear test for wrappersCovers the optional-id path on clear functions.
describe('exported functions', () => { @@ describe('managedClearInterval', () => { @@ }) }) + + it('managedClearTimeout/Interval accept undefined IDs (no-op)', () => { + const ct = vi.spyOn(timeoutManager, 'clearTimeout') + const ci = vi.spyOn(timeoutManager, 'clearInterval') + managedClearTimeout(undefined) + managedClearInterval(undefined) + expect(ct).toHaveBeenCalledWith(undefined) + expect(ci).toHaveBeenCalledWith(undefined) + })Also applies to: 181-191
24-30: Optional: run tests with fake timers for determinism and speedPrevents real timer allocation and OS scheduling variance across environments.
beforeEach(() => { - vi.spyOn(console, 'warn') + vi.useFakeTimers() + vi.spyOn(console, 'warn') }) afterEach(() => { - vi.restoreAllMocks() + vi.restoreAllMocks() + vi.useRealTimers() })
📜 Review details
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.
📒 Files selected for processing (1)
packages/query-core/src/__tests__/timeoutManager.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/query-core/src/__tests__/timeoutManager.test.tsx (1)
packages/query-core/src/timeoutManager.ts (4)
TimeoutManager(62-115)clearTimeout(100-102)clearInterval(112-114)timeoutManager(130-130)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/query-core/src/index.ts (1)
14-26: Optionally re-export TimeoutManager (type or class) for parity and advanced usage.If you want consumers to type against or subclass the manager (similar to other managers), consider exposing it as a type-only export to avoid runtime impact.
Apply if desired:
export { managedSetTimeout, managedClearTimeout, managedSetInterval, managedClearInterval, systemSetTimeoutZero, defaultTimeoutProvider, timeoutManager, type ManagedTimerId, type TimeoutCallback, type TimeoutProvider, type TimeoutProviderId, + type TimeoutManager, } from './timeoutManager'
📜 Review details
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.
📒 Files selected for processing (1)
packages/query-core/src/index.ts(1 hunks)
🔇 Additional comments (2)
packages/query-core/src/index.ts (2)
14-26: Re-export block looks correct; type-only exports keep runtime lean.Names align with the new timeoutManager API and avoid collisions. Good step to make wrappers public.
14-26: Confirm stability of defaultTimeoutProvider and TimeoutProviderId as public API.If these are experimental/internal, consider marking with JSDoc tags or omitting from the root export to keep the surface minimal.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/query-async-storage-persister/src/asyncThrottle.ts (1)
1-1: Good switch to public API import; add workspace dep to keep packages in syncImporting from '@tanstack/query-core' is correct. Please ensure packages/query-async-storage-persister/package.json declares:
"@tanstack/query-core": "workspace:*".Run to verify:
#!/bin/bash rg -n '"@tanstack/query-core"' packages/query-async-storage-persister/package.json || echo "Missing @tanstack/query-core dep" rg -n 'export .*managedSetTimeout' packages/query-core/src/index.ts
🧹 Nitpick comments (2)
packages/query-async-storage-persister/src/asyncThrottle.ts (2)
25-25: Preserve 0ms next-tick semantics when interval <= 0managedSetTimeout is intended for delays > 0. If callers pass interval = 0 (or < 0), route through systemSetTimeoutZero to maintain next-tick behavior.
Apply:
- await new Promise((done) => managedSetTimeout(done, interval)) + await new Promise((done) => + (interval > 0 ? managedSetTimeout : systemSetTimeoutZero)( + done, + Math.max(0, interval), + ), + )Also update the import:
- import { managedSetTimeout } from '@tanstack/query-core' + import { managedSetTimeout, systemSetTimeoutZero } from '@tanstack/query-core'
29-30: LGTM; minor clock nitUsing managedSetTimeout with a strictly positive delay here is correct due to the while (Date.now() < nextExecutionTime) guard. If you ever revisit this, consider a monotonic clock (performance.now()) to avoid wall-clock jumps.
📜 Review details
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.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/query-async-storage-persister/package.json(1 hunks)packages/query-async-storage-persister/src/asyncThrottle.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T17:57:33.139Z
Learnt from: TkDodo
PR: TanStack/query#9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.139Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
packages/query-async-storage-persister/package.jsonpackages/query-async-storage-persister/src/asyncThrottle.ts
🔇 Additional comments (1)
packages/query-async-storage-persister/package.json (1)
62-64: Confirm workspace replacement and consider peerDependency
- Ensure the publish step replaces
workspace:*with the actual released version of@tanstack/query-coreto avoid leaking the workspace protocol.- Optional: to prevent duplicate installs of
query-corein consumer apps, move@tanstack/query-coretopeerDependencies(and retain it indevDependenciesfor local tests).
|
@TkDodo If i was doing this in my codebase, I would add a lint rule that forbids use of |
|
@TkDodo can you approve CI workflow runs for my branch? I don't know if I'm passing CI |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/query-core/src/__tests__/timeoutManager.test.tsx (2)
21-27: Optional: use fake timers to make tests deterministic and avoid real handles.Apply:
beforeEach(() => { - consoleErrorSpy = vi.spyOn(console, 'error') + vi.useFakeTimers() + consoleErrorSpy = vi.spyOn(console, 'error') }) afterEach(() => { - vi.restoreAllMocks() + vi.clearAllTimers() + vi.useRealTimers() + vi.restoreAllMocks() })
97-103: Minor: prefer spy.mockClear() directly.vi.mocked(...) is unnecessary here.
Apply:
- vi.mocked(consoleErrorSpy).mockClear() + consoleErrorSpy.mockClear()packages/query-core/src/timeoutManager.ts (5)
31-49: Avoid passingundefinedinto platform clearers.Forwarding
undefinedtoclearTimeout/clearIntervalcan map to id0in some environments. Make the default provider a no-op onundefined.setTimeout: (callback, delay) => setTimeout(callback, delay), - clearTimeout: (timeoutId) => clearTimeout(timeoutId), + clearTimeout: (timeoutId) => { + if (timeoutId !== undefined) clearTimeout(timeoutId as any) + }, setInterval: (callback, delay) => setInterval(callback, delay), - clearInterval: (intervalId) => clearInterval(intervalId), + clearInterval: (intervalId) => { + if (intervalId !== undefined) clearInterval(intervalId as any) + },
62-62: Remove leftoverOmit<..., 'name'>.
TimeoutProviderhas nonameproperty anymore. UsingOmithere is confusing noise.-export class TimeoutManager implements Omit<TimeoutProvider, 'name'> { +export class TimeoutManager implements TimeoutProvider {
79-97: Tighten the dev warning to give a concrete action.Message is good; adding a brief “what to do” helps users fix it faster.
- console.error( - `[timeoutManager]: Switching provider after calls to previous provider might result in unexpected behavior.`, - { previous: this.#provider, provider }, - ) + console.error( + `[timeoutManager] Provider switched after timers were created. This can break clearing previously scheduled timers. Set the provider once during app initialization before any timers are created.`, + { previousProvider: this.#provider, nextProvider: provider }, + )
128-128: Optional: mark the singleton instantiation as pure.Helps bundlers with tree-shaking in edge cases.
-export const timeoutManager = new TimeoutManager() +export const timeoutManager = /* @__PURE__ */ new TimeoutManager()
1-1: Project-level: add an ESLint guard against raw timers.Recommend enforcing managed timers via lint to prevent regressions.
Add rules similar to:
{ "rules": { "no-restricted-globals": [ "error, { "name": "setTimeout", "message": "Use timeoutManager.setTimeout or systemSetTimeoutZero(0ms) instead." }, { "name": "setInterval", "message": "Use timeoutManager.setInterval instead." } ], "no-restricted-properties": [ "error", { "object": "globalThis", "property": "setTimeout", "message": "Use managed helpers." }, { "object": "globalThis", "property": "setInterval", "message": "Use managed helpers." }, { "object": "window", "property": "setTimeout", "message": "Use managed helpers." }, { "object": "window", "property": "setInterval", "message": "Use managed helpers." } ] } }
📜 Review details
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.
📒 Files selected for processing (9)
packages/query-async-storage-persister/src/asyncThrottle.ts(2 hunks)packages/query-core/src/__tests__/timeoutManager.test.tsx(1 hunks)packages/query-core/src/index.ts(1 hunks)packages/query-core/src/notifyManager.ts(2 hunks)packages/query-core/src/queryObserver.ts(5 hunks)packages/query-core/src/removable.ts(3 hunks)packages/query-core/src/timeoutManager.ts(1 hunks)packages/query-core/src/utils.ts(2 hunks)packages/query-sync-storage-persister/src/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/query-core/src/notifyManager.ts
- packages/query-core/src/queryObserver.ts
- packages/query-sync-storage-persister/src/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T17:57:33.139Z
Learnt from: TkDodo
PR: TanStack/query#9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.139Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
packages/query-async-storage-persister/src/asyncThrottle.ts
🧬 Code graph analysis (5)
packages/query-core/src/timeoutManager.ts (2)
packages/query-core/src/index.ts (4)
TimeoutCallback(25-25)ManagedTimerId(24-24)TimeoutProvider(26-26)timeoutManager(23-23)packages/query-core/src/queryObserver.ts (2)
timeoutManager(414-419)timeoutManager(421-426)
packages/query-core/src/__tests__/timeoutManager.test.tsx (1)
packages/query-core/src/timeoutManager.ts (6)
TimeoutManager(62-126)clearTimeout(112-114)clearInterval(123-125)timeoutManager(128-128)defaultTimeoutProvider(31-49)systemSetTimeoutZero(137-139)
packages/query-core/src/utils.ts (3)
packages/query-core/src/timeoutManager.ts (1)
timeoutManager(128-128)packages/query-core/src/index.ts (1)
timeoutManager(23-23)packages/query-core/src/queryObserver.ts (2)
timeoutManager(414-419)timeoutManager(421-426)
packages/query-async-storage-persister/src/asyncThrottle.ts (3)
packages/query-core/src/timeoutManager.ts (1)
timeoutManager(128-128)packages/query-core/src/index.ts (1)
timeoutManager(23-23)packages/query-core/src/queryObserver.ts (2)
timeoutManager(414-419)timeoutManager(421-426)
packages/query-core/src/removable.ts (2)
packages/query-core/src/timeoutManager.ts (2)
ManagedTimerId(17-17)timeoutManager(128-128)packages/query-core/src/queryObserver.ts (2)
timeoutManager(414-419)timeoutManager(421-426)
🔇 Additional comments (11)
packages/query-core/src/utils.ts (2)
1-1: Import looks good; aligns with the new central timer abstraction.
363-367: Clarify zero-delay behavior
Tests invokesleep(0)extensively and rely on it queuing a macrotask via the fake timer—confirm whethertimeoutManager.setTimeout(resolve, 0)calls the specialsystemSetTimeoutZero(microtask) or the regular timeout path, and adjust providers or docs so zero-delay semantics are explicit.packages/query-async-storage-persister/src/asyncThrottle.ts (2)
1-1: Approve public API import and workspace dependency Verified that@tanstack/query-coreis declared inpackages/query-async-storage-persister/package.json.
25-31: Zero-Interval Behavior Needs Confirmation
No static call sites passinterval: 0(rg found none), but ifintervalcan be zero at runtime, decide whether zero should be treated as a “next tick” viasystemSetTimeoutZero(to bypass provider interception) or retain the current behavior. Confirm the intended semantics and update the implementation or documentation accordingly.packages/query-core/src/removable.ts (1)
16-20: Good migration to timeoutManager-managed timers.packages/query-core/src/__tests__/timeoutManager.test.tsx (1)
122-131: Good assertion for systemSetTimeoutZero; tighten cleanup.You already clear the created timeout via the spy result — nice.
packages/query-core/src/index.ts (1)
22-27: No internal imports found—omit exports if you prefer a minimal API, but verify no external consumers rely on them.packages/query-core/src/timeoutManager.ts (4)
1-9: Callback typing is spot on.The void-arg trick keeps
new Promise((r) => timeoutManager.setTimeout(r, n))type-safe. LGTM.
105-121: Method delegation and dev-only usage tracking look good.State is only toggled under NODE_ENV !== 'production'; no runtime overhead in prod. LGTM.
112-125: clearers delegate correctly.Once the
undefinedguard (above) is in, this section is solid.
137-139: Zero-delay helper is clear and intentional.Explicitly bypassing the manager for next-tick semantics is the right call. LGTM.
| if (provider === this.#provider) { | ||
| return | ||
| } |
There was a problem hiding this comment.
if the point of this early return is to just not have the console.error when called with the same provider, I think we could just move this check into the env check too:
if (process.env.NODE_ENV !== 'production') {
- if (this.#providerCalled) {
+ if (this.#providerCalled && provider !== this.#provider)because it needs to work with whatever scheduleFn we have set there
|
I thought had to export but then it occurred to me that calling so the correct fix (I hope) is to use |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9612 +/- ##
==========================================
+ Coverage 45.27% 45.47% +0.19%
==========================================
Files 208 209 +1
Lines 8342 8372 +30
Branches 1890 1895 +5
==========================================
+ Hits 3777 3807 +30
Misses 4118 4118
Partials 447 447 🚀 New features to boost your workflow:
|
|
bundle size goes from 44.4 (13.2) to 44.9 (13.3), which is acceptable 👍 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/query-core/src/index.ts (3)
14-14: ExportingdefaultScheduler: confirm it’s a supported public APISurfacing
defaultSchedulerexpands the stable API. If it’s an internal detail ofnotifyManager, consider not re-exporting to avoid accidental reliance.
33-41: Value + typeskipTokennaming: doc nudgeExporting both the value
skipTokenand typeSkipTokenis fine in TS, but can confuse consumers. Consider a brief docs note showing correct import patterns.
48-56: Broader API exposure: double-check stability and duplicationRe-exporting
Mutation,Query, and their*Statetypes plusexport * from './types'widens the stable surface. Ensure:
- These are intended as public/stable.
- No duplicate/conflicting type names arise via the star export.
If any are meant to remain internal, consider removing them from
index.ts.
📜 Review details
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.
📒 Files selected for processing (1)
packages/query-core/src/index.ts(1 hunks)
🔇 Additional comments (3)
packages/query-core/src/index.ts (3)
3-9: LGTM:focusManagerand hydration exportsThese re-exports look good and align with existing public surface.
16-21: LGTM: observers/cache/client/cancel exportsThe additions are consistent with prior public entities and won’t surprise consumers.
43-43: Experimental alias: verify policy and docsConfirm
experimental_streamedQueryfollows your established “experimental_” policy across adapters (changelog, docs, no semver guarantees).
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/query-core/src/index.ts (1)
22-27: Public timer surface looks good; confirmsystemSetTimeoutZerois internal and prevent misuse of timer IDsGood call exporting only
timeoutManagerand timer types—keeps the public API minimal and matches earlier feedback. Please confirmsystemSetTimeoutZerois intentionally not exported and that no packages import it fromquery-core.#!/bin/bash # Ensure no consumers rely on a public export of systemSetTimeoutZero rg -nP -g '!**/__tests__/**' -C2 '\bsystemSetTimeoutZero\b' packagesOptional hardening: make
ManagedTimerIda branded type so it can’t be accidentally passed to the globalclearTimeout/clearInterval.Example (in timeoutManager.ts, shown conceptually):
export type ManagedTimerId = { __brand: 'ManagedTimerId' } & { id: unknown }Project-wide lint to steer usage toward managed timers:
{ "rules": { "no-restricted-globals": [ "error, { "name": "setTimeout", "message": "Use managedSetTimeout or timeoutManager." }, { "name": "clearTimeout", "message": "Use timeoutManager." }, { "name": "setInterval", "message": "Use managedSetInterval or timeoutManager." }, { "name": "clearInterval", "message": "Use timeoutManager." } ], "no-restricted-properties": [ "error", { "object": "globalThis", "property": "setTimeout", "message": "Use managedSetTimeout or timeoutManager." }, { "object": "globalThis", "property": "clearTimeout", "message": "Use timeoutManager." }, { "object": "globalThis", "property": "setInterval", "message": "Use managedSetInterval or timeoutManager." }, { "object": "globalThis", "property": "clearInterval", "message": "Use timeoutManager." } ] } }
🧹 Nitpick comments (1)
packages/query-core/src/index.ts (1)
14-14: AliasdefaultScheduleras unstable to avoid public API lock-in
No internal usages ofdefaultSchedulerwere found; replace its direct export with an unstable alias:-export { defaultScheduler, notifyManager } from './notifyManager' +export { notifyManager } from './notifyManager' +export { defaultScheduler as unstable_defaultScheduler } from './notifyManager'
📜 Review details
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.
📒 Files selected for processing (3)
packages/query-core/src/index.ts(1 hunks)packages/query-core/src/timeoutManager.ts(1 hunks)packages/query-persist-client-core/src/createPersister.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/query-core/src/timeoutManager.ts
- packages/query-persist-client-core/src/createPersister.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Preview
🔇 Additional comments (3)
packages/query-core/src/index.ts (3)
32-39: Utils/type exports reshuffle looks fine
Additive and non-breaking as presented.Also applies to: 40-40
47-49: Hydration type export move looks correct
No issues with surfacingDehydrateOptionsalongsideDehydratedState/HydrateOptions.
50-55: Verified no export conflicts
types.ts contains only explicit exports and no wildcard exports; none of its exported names collide with index.ts’s explicit exports. Selective exports remain optional for API surface control.
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
docs/framework/react/plugins/persistQueryClient.md (1)
24-24: Clarify the workaround’s prerequisite.Make explicit that exceeding ~24 days requires installing a custom provider that supports longer delays; the default provider still uses the platform cap.
-Due to a JavaScript limitation, the maximum allowed `gcTime` is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value), although it is possible to work around this limit using [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider). +Due to a JavaScript limitation, the maximum allowed `gcTime` is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value). You can work around this by installing a custom provider via [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider) that supports longer delays.docs/framework/react/reference/useQuery.md (1)
104-106: Tighten wording of the 24‑day note.Minor phrasing tweak for clarity and to mirror other docs.
- - Note: the maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value), although it is possible to work around this limit using [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider). + - Note: platform timers cap delays at about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value). To exceed this, install a custom provider via [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider).docs/reference/timeoutManager.md (4)
61-61: Fix punctuation: add missing space after link.“There should be a space before ‘The’.”
-`setTimeout(callback, delayMs)` schedules a callback to run after approximately `delay` milliseconds, like the global [setTimeout function](https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout).The callback can be canceled with `timeoutManager.clearTimeout`. +`setTimeout(callback, delayMs)` schedules a callback to run after approximately `delay` milliseconds, like the global [setTimeout function](https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout). The callback can be canceled with `timeoutManager.clearTimeout`.
12-19: Document systemSetTimeoutZero alongside other methods.It’s exported and used by notifyManager; include it in the list for completeness.
- [`timeoutManager.setInterval`](#timeoutmanagersetinterval) - [`timeoutManager.clearInterval`](#timeoutmanagerclearinterval) +- [`systemSetTimeoutZero`](#systemsettimeoutzero)
20-36: Add a brief section for systemSetTimeoutZero.Readers need to know when to bypass the manager for a 0ms macrotask.
+## `systemSetTimeoutZero` + +Schedules `callback` with a 0 ms delay on the platform event loop, bypassing `TimeoutManager`. Use this when you specifically need “next macrotask” semantics rather than a managed timer. + +```tsx +import { systemSetTimeoutZero } from '@tanstack/react-query' + +systemSetTimeoutZero(() => { + // runs on the next macrotask tick +}) +```
42-43: Use “Node.js” spelling.Nit on proper casing.
-The `TimeoutProvider` type requires that implementations handle timer ID objects that can be converted to `number` via [Symbol.toPrimitive][toPrimitive] because runtimes like NodeJS return [objects][nodejs-timeout] from their global `setTimeout` and `setInterval` functions. +The `TimeoutProvider` type requires that implementations handle timer ID objects that can be converted to `number` via [Symbol.toPrimitive][toPrimitive] because runtimes like Node.js return [objects][nodejs-timeout] from their global `setTimeout` and `setInterval` functions.docs/framework/solid/reference/useQuery.md (1)
226-228: Mirror React docs phrasing for the 24‑day cap.Keeps consistency across framework docs.
- - Note: the maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value), although it is possible to work around this limit using [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider). + - Note: platform timers cap delays at about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value). To exceed this, install a custom provider via [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider).
📜 Review details
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.
📒 Files selected for processing (6)
docs/config.json(1 hunks)docs/framework/react/plugins/persistQueryClient.md(1 hunks)docs/framework/react/reference/useMutation.md(1 hunks)docs/framework/react/reference/useQuery.md(1 hunks)docs/framework/solid/reference/useQuery.md(1 hunks)docs/reference/timeoutManager.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- docs/framework/react/reference/useMutation.md
- docs/config.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: justjake
PR: TanStack/query#9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.536Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
📚 Learning: 2025-09-04T16:36:04.536Z
Learnt from: justjake
PR: TanStack/query#9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.536Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
Applied to files:
docs/framework/react/plugins/persistQueryClient.mddocs/reference/timeoutManager.md
🪛 LanguageTool
docs/framework/react/reference/useQuery.md
[grammar] ~104-~104: There might be a mistake here.
Context: ...er.md#timeoutmanagersettimeoutprovider). - If set to Infinity, will disable garba...
(QB_NEW_EN)
docs/framework/solid/reference/useQuery.md
[grammar] ~226-~226: There might be a mistake here.
Context: ...er.md#timeoutmanagersettimeoutprovider). - If set to Infinity, will disable garba...
(QB_NEW_EN)
docs/reference/timeoutManager.md
[style] ~23-~23: To form a complete sentence, be sure to include a subject.
Context: ...tTimeoutProvider setTimeoutProvider` can be used to set a custom implementation ...
(MISSING_IT_THERE)
[grammar] ~40-~40: Ensure spelling is correct
Context: ... seconds) tend to be latency sensitive, where long-term timers may benefit more from ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~108-~108: To form a complete sentence, be sure to include a subject.
Context: ...rInterval clearInterval(intervalId)` can be used to cancel an interval, like the...
(MISSING_IT_THERE)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
docs/reference/timeoutManager.md (3)
29-36: Fix import path for timeoutManager (react-query does not re-export it).Import
timeoutManagerfrom@tanstack/query-coreand keepQueryClientfrom@tanstack/react-queryto make the example runnable.-import { timeoutManager, QueryClient } from '@tanstack/react-query' +import { timeoutManager } from '@tanstack/query-core' +import { QueryClient } from '@tanstack/react-query'
80-89: Same import fix here.Repeat the import correction for the clearTimeout example.
-import { timeoutManager } from '@tanstack/react-query' +import { timeoutManager } from '@tanstack/query-core'
98-104: Ensure consistent import source in setInterval example.Use
@tanstack/query-core.-import { timeoutManager } from '@tanstack/react-query' +import { timeoutManager } from '@tanstack/query-core'
🧹 Nitpick comments (5)
docs/reference/timeoutManager.md (5)
61-64: Punctuation and spacing nit.Missing space after the closing parenthesis.
-... setTimeout function](https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout).The callback ... +... setTimeout function](https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout). The callback ...
40-43: Hyphenation and wording polish in performance note.Small readability improvements; no semantic changes.
-Timers are very performance sensitive. Short term timers (such as those with delays less than 5 seconds) tend to be latency sensitive, where long-term timers may benefit more from ... +Timers are very performance-sensitive. Short‑term timers (such as those with delays less than 5 seconds) tend to be latency‑sensitive, whereas long‑term timers may benefit more from ...
47-57: Document TimeoutCallback or inline its shape.
TimeoutProviderreferencesTimeoutCallbackbut the page doesn’t define it. Add a short alias to avoid confusion.```tsx -type ManagedTimerId = number | { [Symbol.toPrimitive]: () => number } +type ManagedTimerId = number | { [Symbol.toPrimitive]: () => number } +type TimeoutCallback = () => void
21-28: Minor style: clarify method list lead-in and add install timing emphasis.
- “Its available methods” → “The available methods”.
- Bold the “before creating a QueryClient” guidance to reduce misconfiguration.
-## `timeoutManager.setTimeoutProvider` - -`setTimeoutProvider` can be used to set a custom implementation of the `setTimeout`, `clearTimeout`, `setInterval`, `clearInterval` functions, called a `TimeoutProvider`. +## `timeoutManager.setTimeoutProvider` + +The available methods allow you to replace the underlying timing APIs with a custom `TimeoutProvider`. + +`setTimeoutProvider` sets a custom implementation of `setTimeout`, `clearTimeout`, `setInterval`, and `clearInterval`, called a `TimeoutProvider`.-It is important to call `setTimeoutProvider` before creating a QueryClient or queries, so that the same provider is used consistently for all timers in the application, since different TimeoutProviders cannot cancel each others' timers. +It is important to call `setTimeoutProvider` **before creating a `QueryClient` or any queries**, so the same provider is used consistently for all timers in the application. Different `TimeoutProvider`s cannot cancel each others' timers.
59-75: Add note about zero-delay timers andsystemSetTimeoutZero.This page introduces a manager that intentionally bypasses provider logic for
setTimeout(cb, 0)to preserve “next tick” semantics. A short note prevents confusion.## `timeoutManager.setTimeout` `setTimeout(callback, delayMs)` schedules a callback to run after approximately `delay` milliseconds, like the global [setTimeout function](https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout). The callback can be canceled with `timeoutManager.clearTimeout`. +Note: zero-delay timers (`delay === 0`) are scheduled using the system event loop to preserve “next tick” semantics. Use the exported helper `systemSetTimeoutZero(callback)` for that case.
📜 Review details
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.
📒 Files selected for processing (1)
docs/reference/timeoutManager.md(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: justjake
PR: TanStack/query#9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.536Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
📚 Learning: 2025-09-04T16:36:04.536Z
Learnt from: justjake
PR: TanStack/query#9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.536Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
Applied to files:
docs/reference/timeoutManager.md
📚 Learning: 2025-09-02T17:57:33.139Z
Learnt from: TkDodo
PR: TanStack/query#9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.139Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
docs/reference/timeoutManager.md
🪛 LanguageTool
docs/reference/timeoutManager.md
[style] ~23-~23: To form a complete sentence, be sure to include a subject.
Context: ...tTimeoutProvider setTimeoutProvider` can be used to set a custom implementation ...
(MISSING_IT_THERE)
[grammar] ~40-~40: Ensure spelling is correct
Context: ... seconds) tend to be latency sensitive, where long-term timers may benefit more from ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~108-~108: To form a complete sentence, be sure to include a subject.
Context: ...rInterval clearInterval(intervalId)` can be used to cancel an interval, like the...
(MISSING_IT_THERE)
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/framework/solid/reference/useQuery.md (1)
228-231: Fix missing closing quote in networkMode union.User-facing docs typo.
- - ##### `networkMode: 'online' | 'always' | 'offlineFirst` + - ##### `networkMode: 'online' | 'always' | 'offlineFirst'`
🧹 Nitpick comments (2)
docs/framework/solid/reference/useQuery.md (1)
226-226: Polish the “Note” sentence for clarity; split into two sentences.Improves readability and should appease the grammar checker.
- - Note: the maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value), although it is possible to work around this limit using [timeoutManager.setTimeoutProvider](../../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider). + - Note: The maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value). If you need a longer delay, you can work around this limit by using [timeoutManager.setTimeoutProvider](../../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider).docs/framework/react/reference/useQuery.md (1)
104-105: Nit: update Note wording for clarity and consistency
Verified the relative link and anchor resolve correctly in docs/reference/timeoutManager.md.- - Note: the maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value), although it is possible to work around this limit using [timeoutManager.setTimeoutProvider](../../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider). + - Note: The maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value). If you need a longer delay, you can work around this limit by using [timeoutManager.setTimeoutProvider](../../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider).
📜 Review details
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.
📒 Files selected for processing (4)
docs/framework/react/plugins/persistQueryClient.md(1 hunks)docs/framework/react/reference/useMutation.md(1 hunks)docs/framework/react/reference/useQuery.md(1 hunks)docs/framework/solid/reference/useQuery.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/framework/react/reference/useMutation.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/framework/react/plugins/persistQueryClient.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: justjake
PR: TanStack/query#9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.536Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
🪛 LanguageTool
docs/framework/solid/reference/useQuery.md
[grammar] ~226-~226: There might be a mistake here.
Context: ...er.md#timeoutmanagersettimeoutprovider). - If set to Infinity, will disable garba...
(QB_NEW_EN)
docs/framework/react/reference/useQuery.md
[grammar] ~104-~104: There might be a mistake here.
Context: ...er.md#timeoutmanagersettimeoutprovider). - If set to Infinity, will disable garba...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/framework/solid/reference/useQuery.md (1)
226-227: Fix relative link and tighten wordingThe relative path to the TimeoutManager docs likely needs one fewer
../. Also small style/clarity polish.- - Note: the maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value), although it is possible to work around this limit using [timeoutManager.setTimeoutProvider](../../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider). + - Note: The maximum allowed time is about [24 days](https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value) (~2^31−1 ms). You can work around this limit by using [timeoutManager.setTimeoutProvider](../../../reference/timeoutManager.md#timeoutmanagersettimeoutprovider).If the three-dot path is wrong for this docs section, please adjust to whichever relative depth your other cross-links to
/reference/timeoutManager.mduse.
📜 Review details
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.
📒 Files selected for processing (1)
docs/framework/solid/reference/useQuery.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: justjake
PR: TanStack/query#9612
File: packages/query-core/src/__tests__/timeoutManager.test.tsx:37-57
Timestamp: 2025-09-04T16:36:04.536Z
Learning: In Node.js v12.19+ (and browsers), converting a Timeout object to a number (e.g., Number(timeoutId)) is valid because Timeout implements Symbol.toPrimitive; clearTimeout/clearInterval accept the object or its numeric primitive. Do not flag this pattern in tests like packages/query-core/src/__tests__/timeoutManager.test.tsx.
🪛 LanguageTool
docs/framework/solid/reference/useQuery.md
[grammar] ~226-~226: There might be a mistake here.
Context: ...er.md#timeoutmanagersettimeoutprovider). - If set to Infinity, will disable garba...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (1)
docs/framework/solid/reference/useQuery.md (1)
228-228: Formatting fix fornetworkModelooks goodBackticks and heading formatting read correctly now.
Introduces new class
TimeoutManagerthat is a facade over a "provider" implementation ofsetTimeout,clearTimeout,setInterval,clearInterval, following the pattern ofOnlineManagerandFocusManager.All calls to global
setTimeout/setIntervaland their matchingclearTimeout/clearIntervalare replaced by new helper functions that useTimeoutManageras appropriate instead:setTimeout(cb, number > 0)->managedSetTimeoutsetTimeout(cb, 0)->systemSetTimeoutZero(to preserve the intention of running on the next system event loop tick)setInterval(cb, number)->managedSetIntervalI opted to export functions instead of do direct calls on
timeoutManager.setTimeoutas this reduces bundle size impact in callers.Let me know what you think.
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation
Chores