Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
204 changes: 204 additions & 0 deletions packages/query-core/src/__tests__/timeoutManager.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import {
TimeoutManager,
defaultTimeoutProvider,
managedClearInterval,
managedClearTimeout,
managedSetInterval,
managedSetTimeout,
systemSetTimeoutZero,
timeoutManager,
} from '../timeoutManager'

describe('timeoutManager', () => {
function createMockProvider(name: string = 'custom') {
return {
name,
setTimeout: vi.fn(() => 123),
clearTimeout: vi.fn(),
setInterval: vi.fn(() => 456),
clearInterval: vi.fn(),
}
}

beforeEach(() => {
vi.spyOn(console, 'warn')
})

afterEach(() => {
vi.restoreAllMocks()
})

describe('TimeoutManager', () => {
let manager: TimeoutManager

beforeEach(() => {
manager = new TimeoutManager()
})

it('by default proxies calls to globalThis setTimeout/clearTimeout', () => {
const setTimeoutSpy = vi.spyOn(globalThis, 'setTimeout')
const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout')
const setIntervalSpy = vi.spyOn(globalThis, 'setInterval')
const clearIntervalSpy = vi.spyOn(globalThis, 'clearInterval')

const callback = vi.fn()
const timeoutId = manager.setTimeout(callback, 100)
expect(setTimeoutSpy).toHaveBeenCalledWith(callback, 100)
clearTimeout(timeoutId)

manager.clearTimeout(200)
expect(clearTimeoutSpy).toHaveBeenCalledWith(200)

const intervalId = manager.setInterval(callback, 300)
expect(setIntervalSpy).toHaveBeenCalledWith(callback, 300)
clearInterval(intervalId)

manager.clearInterval(400)
expect(clearIntervalSpy).toHaveBeenCalledWith(400)
})

describe('setTimeoutProvider', () => {
it('proxies calls to the configured timeout provider', () => {
const customProvider = createMockProvider()
manager.setTimeoutProvider(customProvider)

const callback = vi.fn()

manager.setTimeout(callback, 100)
expect(customProvider.setTimeout).toHaveBeenCalledWith(callback, 100)

manager.clearTimeout(999)
expect(customProvider.clearTimeout).toHaveBeenCalledWith(999)

manager.setInterval(callback, 200)
expect(customProvider.setInterval).toHaveBeenCalledWith(callback, 200)

manager.clearInterval(888)
expect(customProvider.clearInterval).toHaveBeenCalledWith(888)
})

it('warns when switching providers after making call', () => {
// 1. switching before making any calls does not warn
const customProvider = createMockProvider()
manager.setTimeoutProvider(customProvider)
expect(console.warn).not.toHaveBeenCalled()

// Make a call. The next switch should warn
manager.setTimeout(vi.fn(), 100)

// 2. switching after making a call should warn
const customProvider2 = createMockProvider('custom2')
manager.setTimeoutProvider(customProvider2)
expect(console.warn).toHaveBeenCalledWith(
'[timeoutManager]: Switching to custom2 provider after calls to custom provider might result in unexpected behavior.',
)

// 3. Switching again with no intermediate calls should not warn
vi.mocked(console.warn).mockClear()
const customProvider3 = createMockProvider('custom3')
manager.setTimeoutProvider(customProvider3)
expect(console.warn).not.toHaveBeenCalled()
})
})

it('throws if provider returns non-convertible value from setTimeout/setInterval', () => {
const invalidValue = { invalid: true } as any
const customProvider = createMockProvider('badProvider')
customProvider.setTimeout = vi.fn(() => invalidValue)
customProvider.setInterval = vi.fn(() => invalidValue)
manager.setTimeoutProvider(customProvider)

const callback = vi.fn()

expect(() => manager.setTimeout(callback, 100)).toThrow(
'TimeoutManager: could not convert badProvider provider timeout ID to valid number',
)

expect(() => manager.setInterval(callback, 100)).toThrow(
'TimeoutManager: could not convert badProvider provider timeout ID to valid number',
)
})
})

describe('globalThis timeoutManager instance', () => {
it('should be an instance of TimeoutManager', () => {
expect(timeoutManager).toBeInstanceOf(TimeoutManager)
})
})

describe('exported functions', () => {
let provider: ReturnType<typeof createMockProvider>
let callNumber = 0
beforeEach(() => {
callNumber = 0
provider = createMockProvider()
timeoutManager.setTimeoutProvider(provider)
})
afterEach(() => {
timeoutManager.setTimeoutProvider(defaultTimeoutProvider)
})

const callbackArgs = () => [vi.fn(), 100 * ++callNumber] as const

describe('managedSetTimeout', () => {
it('should call timeoutManager.setTimeout', () => {
const spy = vi.spyOn(timeoutManager, 'setTimeout')
const args = callbackArgs()

const result = managedSetTimeout(...args)

expect(spy).toHaveBeenCalledWith(...args)
expect(result).toBe(123)
})
})

describe('managedClearTimeout', () => {
it('should call timeoutManager.clearTimeout', () => {
const spy = vi.spyOn(timeoutManager, 'clearTimeout')
const timeoutId = 123

managedClearTimeout(timeoutId)

expect(spy).toHaveBeenCalledWith(timeoutId)

spy.mockRestore()
})
})

describe('managedSetInterval', () => {
it('should call timeoutManager.setInterval', () => {
const spy = vi.spyOn(timeoutManager, 'setInterval')
const args = callbackArgs()

const result = managedSetInterval(...args)

expect(spy).toHaveBeenCalledWith(...args)
expect(result).toBe(456)
})
})

describe('managedClearInterval', () => {
it('should call timeoutManager.clearInterval', () => {
const spy = vi.spyOn(timeoutManager, 'clearInterval')
const intervalId = 456

managedClearInterval(intervalId)

expect(spy).toHaveBeenCalledWith(intervalId)
})
})

describe('systemSetTimeoutZero', () => {
it('should use globalThis setTimeout with 0 delay', () => {
const spy = vi.spyOn(globalThis, 'setTimeout')

const callback = vi.fn()
systemSetTimeoutZero(callback)

expect(spy).toHaveBeenCalledWith(callback, 0)
clearTimeout(spy.mock.results[0]?.value)
})
})
})
})
13 changes: 13 additions & 0 deletions packages/query-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ 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,
timeoutManager,
type ManagedTimerId,
type TimeoutCallback,
type TimeoutProvider,
type TimeoutProviderId,
} 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
Loading