Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
8fc4d3d
add timeoutManager class
justjake Sep 2, 2025
2fb1f1f
add additional types & export functions
justjake Sep 2, 2025
acb595c
convert all setTimeout/setInterval to managed versions
justjake Sep 2, 2025
96813b6
tweaks
justjake Sep 2, 2025
530504e
add claude-generated tests
justjake Sep 2, 2025
223371b
tests
justjake Sep 2, 2025
d3d7d1a
revert changes in query-async-storage-persister: no path to import qu…
justjake Sep 2, 2025
a953c70
re-export more types
justjake Sep 2, 2025
0fac7b2
console.warn -> non-production console.error
justjake Sep 2, 2025
47962fd
query-async-storage-persister: use query-core managedSetTimeout
justjake Sep 2, 2025
62b1dd0
pdate pnpm-lock for new dependency edge
justjake Sep 2, 2025
8d5e050
sleep: always managedSetTimeout
justjake Sep 2, 2025
fea6cce
remove managed* functions, call method directly
justjake Sep 3, 2025
1606b58
remove runtime coercion and accept unsafe any within TimeoutManager c…
justjake Sep 3, 2025
fc9092b
cleanup; fix test after changes
justjake Sep 3, 2025
5d6fe4d
name is __TEST_ONLY__
justjake Sep 3, 2025
ad1fb2b
notifyManager: default scheduler === systemSetTimeoutZero
justjake Sep 3, 2025
932c3a2
Improve TimeoutCallback comment since ai was confused
justjake Sep 3, 2025
a6d38f8
remove unnecessary timeoutManager-related exports
justjake Sep 3, 2025
81d35ac
prettier-ify index.ts (seems my editor messed with it already this pr?)
justjake Sep 3, 2025
b6fffb4
continue to export defaultTimeoutProvider for tests
justjake Sep 3, 2025
948c646
oops missing import
justjake Sep 3, 2025
08a2c5f
Merge branch 'main' into jake--timeoutmanager
TkDodo Sep 4, 2025
841ac54
fix: export systemSetTimeoutZero from core
TkDodo Sep 4, 2025
fb22c67
ref: use notifyManager.schedule in createPersister
TkDodo Sep 4, 2025
09a787e
ref: move provider check behind env check
TkDodo Sep 4, 2025
7fb57b1
docs
justjake Sep 4, 2025
4b1e8af
doc tweaks
justjake Sep 4, 2025
b6ca80d
doc tweaks
justjake Sep 4, 2025
73014f5
docs: reference timeoutManager in discussion of 24 day setTimout limit
justjake Sep 4, 2025
3f452ea
Apply suggestion from @TkDodo
TkDodo Sep 5, 2025
cca861f
Apply suggestion from @TkDodo
TkDodo Sep 5, 2025
d58e28f
chore: fix broken links
TkDodo Sep 5, 2025
7922966
docs: syntax fix
TkDodo Sep 5, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/query-async-storage-persister/src/asyncThrottle.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { managedSetTimeout } from '../../query-core/src/timeoutManager'
import { noop } from './utils'

interface AsyncThrottleOptions {
Expand All @@ -21,11 +22,11 @@ export function asyncThrottle<TArgs extends ReadonlyArray<unknown>>(
if (isScheduled) return
isScheduled = true
while (isExecuting) {
await new Promise((done) => setTimeout(done, interval))
await new Promise((done) => managedSetTimeout(done, interval))
}
while (Date.now() < nextExecutionTime) {
await new Promise((done) =>
setTimeout(done, nextExecutionTime - Date.now()),
managedSetTimeout(done, nextExecutionTime - Date.now()),
)
}
isScheduled = false
Expand Down
9 changes: 9 additions & 0 deletions packages/query-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ export { MutationCache } from './mutationCache'
export type { MutationCacheNotifyEvent } from './mutationCache'
export { MutationObserver } from './mutationObserver'
export { notifyManager, defaultScheduler } from './notifyManager'
export {
managedSetTimeout,
managedClearTimeout,
managedSetInterval,
managedClearInterval,
systemSetTimeoutZero,
defaultTimeoutProvider,
type ManagedTimerId,
} from './timeoutManager'
export { focusManager } from './focusManager'
export { onlineManager } from './onlineManager'
export {
Expand Down
5 changes: 4 additions & 1 deletion packages/query-core/src/notifyManager.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// TYPES

import { systemSetTimeoutZero } from './timeoutManager'

type NotifyCallback = () => void

type NotifyFunction = (callback: () => void) => void
Expand All @@ -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) =>
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why I have it calling systemSetTimeoutZero - no change in semantics, and the code clearly shows that it's not an oversight

Copy link
Collaborator

Choose a reason for hiding this comment

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

so you’re saying places where we now use systemSetTimeoutZero we should in the future use scheduleMicroTask ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah, I’m just now realizing that systemSetTimeoutZero doesn’t use the timeoutManager 😅 . I get it now 👍

Copy link
Contributor Author

@justjake justjake Sep 3, 2025

Choose a reason for hiding this comment

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

so you’re saying places where we now use systemSetTimeoutZero we should in the future use scheduleMicroTask ?

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.

systemSetTimeoutZero(cb)

export function createNotifyManager() {
let queue: Array<NotifyCallback> = []
Expand Down
19 changes: 13 additions & 6 deletions packages/query-core/src/queryObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ import {
shallowEqualObjects,
timeUntilStale,
} from './utils'
import {
managedClearInterval,
managedClearTimeout,
managedSetInterval,
managedSetTimeout,
} from './timeoutManager'
import type { ManagedTimerId } from './timeoutManager'
import type { FetchOptions, Query, QueryState } from './query'
import type { QueryClient } from './queryClient'
import type { PendingThenable, Thenable } from './thenable'
Expand Down Expand Up @@ -62,8 +69,8 @@ export class QueryObserver<
// This property keeps track of the last query with defined data.
// It will be used to pass the previous data and query to the placeholder function between renders.
#lastQueryWithDefinedData?: Query<TQueryFnData, TError, TQueryData, TQueryKey>
#staleTimeoutId?: ReturnType<typeof setTimeout>
#refetchIntervalId?: ReturnType<typeof setInterval>
#staleTimeoutId?: ManagedTimerId
#refetchIntervalId?: ManagedTimerId
#currentRefetchInterval?: number | false
#trackedProps = new Set<keyof QueryObserverResult>()

Expand Down Expand Up @@ -365,7 +372,7 @@ export class QueryObserver<
// To mitigate this issue we always add 1 ms to the timeout.
const timeout = time + 1

this.#staleTimeoutId = setTimeout(() => {
this.#staleTimeoutId = managedSetTimeout(() => {
if (!this.#currentResult.isStale) {
this.updateResult()
}
Expand Down Expand Up @@ -394,7 +401,7 @@ export class QueryObserver<
return
}

this.#refetchIntervalId = setInterval(() => {
this.#refetchIntervalId = managedSetInterval(() => {
if (
this.options.refetchIntervalInBackground ||
focusManager.isFocused()
Expand All @@ -411,14 +418,14 @@ export class QueryObserver<

#clearStaleTimeout(): void {
if (this.#staleTimeoutId) {
clearTimeout(this.#staleTimeoutId)
managedClearTimeout(this.#staleTimeoutId)
this.#staleTimeoutId = undefined
}
}

#clearRefetchInterval(): void {
if (this.#refetchIntervalId) {
clearInterval(this.#refetchIntervalId)
managedClearInterval(this.#refetchIntervalId)
this.#refetchIntervalId = undefined
}
}
Expand Down
8 changes: 5 additions & 3 deletions packages/query-core/src/removable.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { managedClearTimeout, managedSetTimeout } from './timeoutManager'
import { isServer, isValidTimeout } from './utils'
import type { ManagedTimerId } from './timeoutManager'

export abstract class Removable {
gcTime!: number
#gcTimeout?: ReturnType<typeof setTimeout>
#gcTimeout?: ManagedTimerId

destroy(): void {
this.clearGcTimeout()
Expand All @@ -12,7 +14,7 @@ export abstract class Removable {
this.clearGcTimeout()

if (isValidTimeout(this.gcTime)) {
this.#gcTimeout = setTimeout(() => {
this.#gcTimeout = managedSetTimeout(() => {
this.optionalRemove()
}, this.gcTime)
}
Expand All @@ -28,7 +30,7 @@ export abstract class Removable {

protected clearGcTimeout() {
if (this.#gcTimeout) {
clearTimeout(this.#gcTimeout)
managedClearTimeout(this.#gcTimeout)
this.#gcTimeout = undefined
}
}
Expand Down
174 changes: 174 additions & 0 deletions packages/query-core/src/timeoutManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/**
* Timeout manager does not support passing arguments to the callback.
* (`void` is the argument type inferred by TypeScript's default typings for `setTimeout(cb, number)`)
*/
export type TimeoutCallback = (_: void) => void

/**
* Wrapping `setTimeout` is awkward from a typing perspective because platform
* typings may extend the return type of `setTimeout`. For example, NodeJS
* typings add `NodeJS.Timeout`; but a non-default `timeoutManager` may not be
* able to return such a type.
*
* Still, we can downlevel `NodeJS.Timeout` to `number` as it implements
* Symbol.toPrimitive.
*/
export type TimeoutProviderId = number | { [Symbol.toPrimitive]: () => number }

/**
* Backend for timer functions.
*/
export type TimeoutProvider = {
/** Used in error messages. */
readonly name: string

readonly setTimeout: (
callback: TimeoutCallback,
delay: number,
) => TimeoutProviderId
readonly clearTimeout: (timeoutId: number | undefined) => void

readonly setInterval: (
callback: TimeoutCallback,
delay: number,
) => TimeoutProviderId
readonly clearInterval: (intervalId: number | undefined) => void
}

export const defaultTimeoutProvider: TimeoutProvider = {
name: 'default',

setTimeout: (callback, delay) => setTimeout(callback, delay),
clearTimeout: (timeoutId) => clearTimeout(timeoutId),

setInterval: (callback, delay) => setInterval(callback, delay),
clearInterval: (intervalId) => clearInterval(intervalId),
}

/** Timeout ID returned by {@link TimeoutManager} */
export type ManagedTimerId = number

/**
* Allows customization of how timeouts are created.
*
* @tanstack/query-core makes liberal use of timeouts to implement `staleTime`
* and `gcTime`. The default TimeoutManager provider uses the platform's global
* `setTimeout` implementation, which is known to have scalability issues with
* thousands of timeouts on the event loop.
*
* If you hit this limitation, consider providing a custom TimeoutProvider that
* coalesces timeouts.
*/
export class TimeoutManager implements Omit<TimeoutProvider, 'name'> {
#provider: TimeoutProvider = defaultTimeoutProvider
#providerCalled = false

setTimeoutProvider(provider: TimeoutProvider): void {
if (provider === this.#provider) {
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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)

Copy link
Collaborator

Choose a reason for hiding this comment

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

did that here: 09a787e


if (this.#providerCalled) {
// After changing providers, `clearTimeout` will not work as expected for
// timeouts from the previous provider.
//
// Since they may allocate the same timeout ID, clearTimeout may cancel an
// arbitrary different timeout, or unexpected no-op.
//
// We could protect against this by mixing the timeout ID bits
// deterministically with some per-provider bits.
//
// We could internally queue `setTimeout` calls to `TimeoutManager` until
// some API call to set the initial provider.
console.warn(
`[timeoutManager]: Switching to ${provider.name} provider after calls to ${this.#provider.name} provider might result in unexpected behavior.`,
)
}

this.#provider = provider
this.#providerCalled = false
}

setTimeout(callback: TimeoutCallback, delay: number): ManagedTimerId {
this.#providerCalled = true
return providerIdToNumber(
this.#provider,
this.#provider.setTimeout(callback, delay),
)
}

clearTimeout(timeoutId: ManagedTimerId | undefined): void {
this.#provider.clearTimeout(timeoutId)
}

setInterval(callback: TimeoutCallback, delay: number): ManagedTimerId {
this.#providerCalled = true
return providerIdToNumber(
this.#provider,
this.#provider.setInterval(callback, delay),
)
}

clearInterval(intervalId: ManagedTimerId | undefined): void {
this.#provider.clearInterval(intervalId)
}
}

function providerIdToNumber(
provider: TimeoutProvider,
providerId: TimeoutProviderId,
): ManagedTimerId {
const numberId = Number(providerId)
if (isNaN(numberId)) {
throw new Error(
`TimeoutManager: could not convert ${provider.name} provider timeout ID to valid number`,
)
}
return numberId
}

export const timeoutManager = new TimeoutManager()

// Exporting functions that use `setTimeout` to reduce bundle size impact, since
// method names on objects are usually not minified.

/** A version of `setTimeout` controlled by {@link timeoutManager}. */
export function managedSetTimeout(
callback: TimeoutCallback,
delay: number,
): ManagedTimerId {
return timeoutManager.setTimeout(callback, delay)
}

/** A version of `clearTimeout` controlled by {@link timeoutManager}. */
export function managedClearTimeout(
timeoutId: ManagedTimerId | undefined,
): void {
timeoutManager.clearTimeout(timeoutId)
}

/** A version of `setInterval` controlled by {@link timeoutManager}. */
export function managedSetInterval(
callback: TimeoutCallback,
delay: number,
): ManagedTimerId {
return timeoutManager.setInterval(callback, delay)
}

/** A version of `clearInterval` controlled by {@link timeoutManager}. */
export function managedClearInterval(
intervalId: ManagedTimerId | undefined,
): void {
timeoutManager.clearInterval(intervalId)
}

/**
* In many cases code wants to delay to the next event loop tick; this is not
* mediated by {@link timeoutManager}.
*
* This function is provided to make auditing the `tanstack/query-core` for
* incorrect use of system `setTimeout` easier.
*/
export function systemSetTimeoutZero(callback: TimeoutCallback): void {
setTimeout(callback, 0)
}
5 changes: 4 additions & 1 deletion packages/query-core/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { managedSetTimeout, systemSetTimeoutZero } from './timeoutManager'
import type {
DefaultError,
Enabled,
Expand Down Expand Up @@ -361,7 +362,9 @@ function hasObjectPrototype(o: any): boolean {

export function sleep(timeout: number): Promise<void> {
return new Promise((resolve) => {
setTimeout(resolve, timeout)
const setTimeoutFn =
timeout === 0 ? systemSetTimeoutZero : managedSetTimeout
setTimeoutFn(resolve, timeout)
})
}

Expand Down
13 changes: 9 additions & 4 deletions packages/query-persist-client-core/src/createPersister.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { hashKey, matchQuery, partialMatchKey } from '@tanstack/query-core'
import {
hashKey,
matchQuery,
partialMatchKey,
systemSetTimeoutZero,
} from '@tanstack/query-core'
import type {
Query,
QueryClient,
Expand Down Expand Up @@ -125,7 +130,7 @@ export function experimental_createQueryPersister<TStorageValue = string>({
} else {
if (afterRestoreMacroTask) {
// Just after restoring we want to get fresh data from the server if it's stale
setTimeout(() => afterRestoreMacroTask(persistedQuery), 0)
systemSetTimeoutZero(() => afterRestoreMacroTask(persistedQuery))
}
// We must resolve the promise here, as otherwise we will have `loading` state in the app until `queryFn` resolves
return persistedQuery.state.data as T
Expand Down Expand Up @@ -213,9 +218,9 @@ export function experimental_createQueryPersister<TStorageValue = string>({

if (matchesFilter && storage != null) {
// Persist if we have storage defined, we use timeout to get proper state to be persisted
setTimeout(() => {
systemSetTimeoutZero(() => {
persistQuery(query)
}, 0)
})
}

return Promise.resolve(queryFnResult)
Expand Down
6 changes: 4 additions & 2 deletions packages/query-sync-storage-persister/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { managedSetTimeout } from '@tanstack/query-core'
import { noop } from './utils'
import type { ManagedTimerId } from '@tanstack/query-core'
import type {
PersistRetryer,
PersistedClient,
Expand Down Expand Up @@ -100,12 +102,12 @@ function throttle<TArgs extends Array<any>>(
func: (...args: TArgs) => any,
wait = 100,
) {
let timer: ReturnType<typeof setTimeout> | null = null
let timer: ManagedTimerId | null = null
let params: TArgs
return function (...args: TArgs) {
params = args
if (timer === null) {
timer = setTimeout(() => {
timer = managedSetTimeout(() => {
func(...params)
timer = null
}, wait)
Expand Down
Loading