From bd7bf49f30e08b8e4ea97d669a8f5c55a793a8f0 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 08:51:53 +0100 Subject: [PATCH 01/25] fix: move staleTime from queryObserver to query as it doesn't make much sense to have different stale times for the same query --- packages/query-core/src/query.ts | 84 ++++++++++++++++++------ packages/query-core/src/queryCache.ts | 5 +- packages/query-core/src/queryClient.ts | 10 +-- packages/query-core/src/queryObserver.ts | 64 +----------------- packages/query-core/src/types.ts | 12 ++-- packages/query-core/src/utils.ts | 4 +- 6 files changed, 80 insertions(+), 99 deletions(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 3485d8ce28..30ea74ccaa 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -1,8 +1,11 @@ import { ensureQueryFn, + isServer, + isValidTimeout, noop, replaceData, resolveEnabled, + resolveStaleTime, skipToken, timeUntilStale, } from './utils' @@ -171,20 +174,29 @@ export class Query< observers: Array> #defaultOptions?: QueryOptions #abortSignalConsumed: boolean + #staleTimeoutId?: ReturnType constructor(config: QueryConfig) { super() this.#abortSignalConsumed = false this.#defaultOptions = config.defaultOptions - this.setOptions(config.options) + this.#initOptions(config.options) this.observers = [] this.#cache = config.cache this.queryKey = config.queryKey this.queryHash = config.queryHash this.#initialState = getDefaultState(this.options) this.state = config.state ?? this.#initialState + this.scheduleGc() + + const nextStaleTime = resolveStaleTime(this.options.staleTime, this) + if (nextStaleTime === undefined || nextStaleTime === 0) { + this.#initialState.isInvalidated = true + } else { + this.#updateStaleTimeout(nextStaleTime) + } } get meta(): QueryMeta | undefined { return this.options.meta @@ -194,7 +206,7 @@ export class Query< return this.#retryer?.promise } - setOptions( + #initOptions( options?: QueryOptions, ): void { this.options = { ...this.#defaultOptions, ...options } @@ -202,6 +214,21 @@ export class Query< this.updateGcTime(this.options.gcTime) } + setOptions( + nextOptions?: QueryOptions, + ): void { + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + const prevStaleTime = this.options?.staleTime + this.#initOptions(nextOptions) + + const nextStaleTime = resolveStaleTime(nextOptions?.staleTime, this) + + // Update stale interval if needed + if (nextStaleTime !== resolveStaleTime(prevStaleTime, this)) { + this.#updateStaleTimeout(nextStaleTime) + } + } + protected optionalRemove() { if (!this.observers.length && this.state.fetchStatus === 'idle') { this.#cache.remove(this) @@ -241,6 +268,7 @@ export class Query< destroy(): void { super.destroy() + this.#clearStaleTimeout() this.cancel({ silent: true }) } @@ -267,25 +295,7 @@ export class Query< } isStale(): boolean { - if (this.state.isInvalidated) { - return true - } - - if (this.getObserversCount() > 0) { - return this.observers.some( - (observer) => observer.getCurrentResult().isStale, - ) - } - - return this.state.data === undefined - } - - isStaleByTime(staleTime = 0): boolean { - return ( - this.state.isInvalidated || - this.state.data === undefined || - !timeUntilStale(this.state.dataUpdatedAt, staleTime) - ) + return this.state.isInvalidated } onFocus(): void { @@ -614,6 +624,13 @@ export class Query< this.state = reducer(this.state) + const nextStaleTime = resolveStaleTime(this.options.staleTime, this) + if (nextStaleTime === undefined || nextStaleTime === 0) { + this.state.isInvalidated = true + } else if (!this.isStale()) { + this.#updateStaleTimeout(nextStaleTime) + } + notifyManager.batch(() => { this.observers.forEach((observer) => { observer.onQueryUpdate() @@ -622,6 +639,31 @@ export class Query< this.#cache.notify({ query: this, type: 'updated', action }) }) } + + #updateStaleTimeout(staleTime = 0): void { + this.#clearStaleTimeout() + + if (isServer || this.isStale() || !isValidTimeout(staleTime)) { + return + } + + const time = timeUntilStale(this.state.dataUpdatedAt, staleTime) + + // The timeout is sometimes triggered 1 ms before the stale time expiration. + // To mitigate this issue we always add 1 ms to the timeout. + const timeout = time + 1 + + this.#staleTimeoutId = setTimeout(() => { + this.invalidate() + }, timeout) + } + + #clearStaleTimeout(): void { + if (this.#staleTimeoutId) { + clearTimeout(this.#staleTimeoutId) + this.#staleTimeoutId = undefined + } + } } export function fetchState< diff --git a/packages/query-core/src/queryCache.ts b/packages/query-core/src/queryCache.ts index 8f637e0a39..16b6c83c20 100644 --- a/packages/query-core/src/queryCache.ts +++ b/packages/query-core/src/queryCache.ts @@ -114,17 +114,20 @@ export class QueryCache extends Subscribable { const queryHash = options.queryHash ?? hashQueryKeyByOptions(queryKey, options) let query = this.get(queryHash) + const defaultedOptions = client.defaultQueryOptions(options) if (!query) { query = new Query({ cache: this, queryKey, queryHash, - options: client.defaultQueryOptions(options), + options: defaultedOptions, state, defaultOptions: client.getQueryDefaults(queryKey), }) this.add(query) + } else { + query.setOptions(defaultedOptions) } return query diff --git a/packages/query-core/src/queryClient.ts b/packages/query-core/src/queryClient.ts index 49032a8802..d342f39934 100644 --- a/packages/query-core/src/queryClient.ts +++ b/packages/query-core/src/queryClient.ts @@ -4,7 +4,6 @@ import { hashQueryKeyByOptions, noop, partialMatchKey, - resolveStaleTime, skipToken, } from './utils' import { QueryCache } from './queryCache' @@ -144,10 +143,7 @@ export class QueryClient { const defaultedOptions = this.defaultQueryOptions(options) const query = this.#queryCache.build(this, defaultedOptions) - if ( - options.revalidateIfStale && - query.isStaleByTime(resolveStaleTime(defaultedOptions.staleTime, query)) - ) { + if (options.revalidateIfStale && query.isStale()) { void this.prefetchQuery(defaultedOptions) } @@ -347,9 +343,7 @@ export class QueryClient { const query = this.#queryCache.build(this, defaultedOptions) - return query.isStaleByTime( - resolveStaleTime(defaultedOptions.staleTime, query), - ) + return query.isStale() ? query.fetch(defaultedOptions) : Promise.resolve(query.state.data as TData) } diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 3dc751f5cd..9be6cd79a6 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -9,9 +9,7 @@ import { noop, replaceData, resolveEnabled, - resolveStaleTime, shallowEqualObjects, - timeUntilStale, } from './utils' import type { FetchOptions, Query, QueryState } from './query' import type { QueryClient } from './queryClient' @@ -66,7 +64,6 @@ 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 - #staleTimeoutId?: ReturnType #refetchIntervalId?: ReturnType #currentRefetchInterval?: number | false #trackedProps = new Set() @@ -138,7 +135,6 @@ export class QueryObserver< destroy(): void { this.listeners = new Set() - this.#clearStaleTimeout() this.#clearRefetchInterval() this.#currentQuery.removeObserver(this) } @@ -202,18 +198,6 @@ export class QueryObserver< // Update result this.updateResult(notifyOptions) - // Update stale interval if needed - if ( - mounted && - (this.#currentQuery !== prevQuery || - resolveEnabled(this.options.enabled, this.#currentQuery) !== - resolveEnabled(prevOptions.enabled, this.#currentQuery) || - resolveStaleTime(this.options.staleTime, this.#currentQuery) !== - resolveStaleTime(prevOptions.staleTime, this.#currentQuery)) - ) { - this.#updateStaleTimeout() - } - const nextRefetchInterval = this.#computeRefetchInterval() // Update refetch interval if needed @@ -355,30 +339,6 @@ export class QueryObserver< return promise } - #updateStaleTimeout(): void { - this.#clearStaleTimeout() - const staleTime = resolveStaleTime( - this.options.staleTime, - this.#currentQuery, - ) - - if (isServer || this.#currentResult.isStale || !isValidTimeout(staleTime)) { - return - } - - const time = timeUntilStale(this.#currentResult.dataUpdatedAt, staleTime) - - // The timeout is sometimes triggered 1 ms before the stale time expiration. - // To mitigate this issue we always add 1 ms to the timeout. - const timeout = time + 1 - - this.#staleTimeoutId = setTimeout(() => { - if (!this.#currentResult.isStale) { - this.updateResult() - } - }, timeout) - } - #computeRefetchInterval() { return ( (typeof this.options.refetchInterval === 'function' @@ -412,17 +372,9 @@ export class QueryObserver< } #updateTimers(): void { - this.#updateStaleTimeout() this.#updateRefetchInterval(this.#computeRefetchInterval()) } - #clearStaleTimeout(): void { - if (this.#staleTimeoutId) { - clearTimeout(this.#staleTimeoutId) - this.#staleTimeoutId = undefined - } - } - #clearRefetchInterval(): void { if (this.#refetchIntervalId) { clearInterval(this.#refetchIntervalId) @@ -589,7 +541,7 @@ export class QueryObserver< isPaused: newState.fetchStatus === 'paused', isPlaceholderData, isRefetchError: isError && hasData, - isStale: isStale(query, options), + isStale: query.isStale(), refetch: this.refetch, promise: this.#currentThenable, } @@ -790,7 +742,7 @@ function shouldFetchOn( if (resolveEnabled(options.enabled, query) !== false) { const value = typeof field === 'function' ? field(query) : field - return value === 'always' || (value !== false && isStale(query, options)) + return value === 'always' || (value !== false && query.isStale()) } return false } @@ -805,17 +757,7 @@ function shouldFetchOptionally( (query !== prevQuery || resolveEnabled(prevOptions.enabled, query) === false) && (!options.suspense || query.state.status !== 'error') && - isStale(query, options) - ) -} - -function isStale( - query: Query, - options: QueryObserverOptions, -): boolean { - return ( - resolveEnabled(options.enabled, query) !== false && - query.isStaleByTime(resolveStaleTime(options.staleTime, query)) + query.isStale() ) } diff --git a/packages/query-core/src/types.ts b/packages/query-core/src/types.ts index b13e52d16c..a903d52f7e 100644 --- a/packages/query-core/src/types.ts +++ b/packages/query-core/src/types.ts @@ -193,6 +193,12 @@ export interface QueryOptions< * Setting it to `Infinity` will disable garbage collection. */ gcTime?: number + /** + * The time in milliseconds after data is considered stale. + * If set to `Infinity`, the data will never be considered stale. + * If set to a function, the function will be executed with the query to compute a `staleTime`. + */ + staleTime?: StaleTime queryFn?: QueryFunction | SkipToken persister?: QueryPersister< NoInfer, @@ -275,12 +281,6 @@ export interface QueryObserverOptions< * Defaults to `true`. */ enabled?: Enabled - /** - * The time in milliseconds after data is considered stale. - * If set to `Infinity`, the data will never be considered stale. - * If set to a function, the function will be executed with the query to compute a `staleTime`. - */ - staleTime?: StaleTime /** * If set to a number, the query will continuously refetch at this frequency in milliseconds. * If set to a function, the function will be executed with the latest data and query to compute a frequency diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index d513780640..c00aacee4f 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -85,8 +85,8 @@ export function isValidTimeout(value: unknown): value is number { return typeof value === 'number' && value >= 0 && value !== Infinity } -export function timeUntilStale(updatedAt: number, staleTime?: number): number { - return Math.max(updatedAt + (staleTime || 0) - Date.now(), 0) +export function timeUntilStale(updatedAt: number, staleTime: number): number { + return Math.max(updatedAt + staleTime - Date.now(), 0) } export function resolveStaleTime< From 22f68d21d2736f9daaa99013e8f50c24c8ed7e93 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:11:01 +0100 Subject: [PATCH 02/25] fix: do not fetch disabled observers on mount this was part of isStale() before, so it got lost in the refactoring --- packages/query-core/src/queryObserver.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 9be6cd79a6..02f0c1f331 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -757,6 +757,7 @@ function shouldFetchOptionally( (query !== prevQuery || resolveEnabled(prevOptions.enabled, query) === false) && (!options.suspense || query.state.status !== 'error') && + resolveEnabled(options.enabled, query) !== false && query.isStale() ) } From 36f53e7a810b78559e859aa6070fdb17c2e16178 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:12:22 +0100 Subject: [PATCH 03/25] refactor: always resolve staleTime to a number --- packages/query-core/src/query.ts | 8 +++++--- packages/query-core/src/utils.ts | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 30ea74ccaa..84a42542f9 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -192,7 +192,7 @@ export class Query< this.scheduleGc() const nextStaleTime = resolveStaleTime(this.options.staleTime, this) - if (nextStaleTime === undefined || nextStaleTime === 0) { + if (nextStaleTime === 0) { this.#initialState.isInvalidated = true } else { this.#updateStaleTimeout(nextStaleTime) @@ -625,7 +625,7 @@ export class Query< this.state = reducer(this.state) const nextStaleTime = resolveStaleTime(this.options.staleTime, this) - if (nextStaleTime === undefined || nextStaleTime === 0) { + if (nextStaleTime === 0) { this.state.isInvalidated = true } else if (!this.isStale()) { this.#updateStaleTimeout(nextStaleTime) @@ -640,9 +640,11 @@ export class Query< }) } - #updateStaleTimeout(staleTime = 0): void { + #updateStaleTimeout(staleTime: number): void { this.#clearStaleTimeout() + console.log('setting stale timer', staleTime) + if (isServer || this.isStale() || !isValidTimeout(staleTime)) { return } diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index c00aacee4f..aef9a61fe8 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -97,8 +97,8 @@ export function resolveStaleTime< >( staleTime: undefined | StaleTime, query: Query, -): number | undefined { - return typeof staleTime === 'function' ? staleTime(query) : staleTime +): number { + return typeof staleTime === 'function' ? staleTime(query) : (staleTime ?? 0) } export function resolveEnabled< From d6ff6927de5032d9149798db6f462c12168f55b1 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:14:54 +0100 Subject: [PATCH 04/25] refactor: move resolveStaleTime into the query itself --- packages/query-core/src/query.ts | 16 +++++++++++----- packages/query-core/src/utils.ts | 13 ------------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 84a42542f9..5b12535700 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -5,7 +5,6 @@ import { noop, replaceData, resolveEnabled, - resolveStaleTime, skipToken, timeUntilStale, } from './utils' @@ -25,6 +24,7 @@ import type { QueryOptions, QueryStatus, SetDataOptions, + StaleTime, } from './types' import type { QueryCache } from './queryCache' import type { QueryObserver } from './queryObserver' @@ -191,7 +191,7 @@ export class Query< this.scheduleGc() - const nextStaleTime = resolveStaleTime(this.options.staleTime, this) + const nextStaleTime = this.#resolveStaleTime(this.options.staleTime) if (nextStaleTime === 0) { this.#initialState.isInvalidated = true } else { @@ -221,10 +221,10 @@ export class Query< const prevStaleTime = this.options?.staleTime this.#initOptions(nextOptions) - const nextStaleTime = resolveStaleTime(nextOptions?.staleTime, this) + const nextStaleTime = this.#resolveStaleTime(nextOptions?.staleTime) // Update stale interval if needed - if (nextStaleTime !== resolveStaleTime(prevStaleTime, this)) { + if (nextStaleTime !== this.#resolveStaleTime(prevStaleTime)) { this.#updateStaleTimeout(nextStaleTime) } } @@ -624,7 +624,7 @@ export class Query< this.state = reducer(this.state) - const nextStaleTime = resolveStaleTime(this.options.staleTime, this) + const nextStaleTime = this.#resolveStaleTime(this.options.staleTime) if (nextStaleTime === 0) { this.state.isInvalidated = true } else if (!this.isStale()) { @@ -640,6 +640,12 @@ export class Query< }) } + #resolveStaleTime( + staleTime: StaleTime | undefined, + ): number { + return typeof staleTime === 'function' ? staleTime(this) : (staleTime ?? 0) + } + #updateStaleTimeout(staleTime: number): void { this.#clearStaleTimeout() diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index aef9a61fe8..763fdd7347 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -7,7 +7,6 @@ import type { QueryFunction, QueryKey, QueryOptions, - StaleTime, } from './types' import type { Mutation } from './mutation' import type { FetchOptions, Query } from './query' @@ -89,18 +88,6 @@ export function timeUntilStale(updatedAt: number, staleTime: number): number { return Math.max(updatedAt + staleTime - Date.now(), 0) } -export function resolveStaleTime< - TQueryFnData = unknown, - TError = DefaultError, - TData = TQueryFnData, - TQueryKey extends QueryKey = QueryKey, ->( - staleTime: undefined | StaleTime, - query: Query, -): number { - return typeof staleTime === 'function' ? staleTime(query) : (staleTime ?? 0) -} - export function resolveEnabled< TQueryFnData = unknown, TError = DefaultError, From 6133fb6a0a57883de3fe0386907e9f645e945b57 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:17:00 +0100 Subject: [PATCH 05/25] refactor: make 0 per default an invalid timeout we use `isValidTimeout` in 3 places: - gcTime: here, we want 0 to trigger a setTimeout, because otherwise, we don't cleanup - staleTime: 0 should be invalid because with 0, we instantly mark queries as invalidated - refetchInterval: 0 was never a valid timer (it's treated the same as false); we had an exception implemented here 2/3 cases want it to _not_ be valid, so that should be the default --- packages/query-core/src/queryObserver.ts | 3 +-- packages/query-core/src/removable.ts | 2 +- packages/query-core/src/utils.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 02f0c1f331..c5aec59e7d 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -355,8 +355,7 @@ export class QueryObserver< if ( isServer || resolveEnabled(this.options.enabled, this.#currentQuery) === false || - !isValidTimeout(this.#currentRefetchInterval) || - this.#currentRefetchInterval === 0 + !isValidTimeout(this.#currentRefetchInterval) ) { return } diff --git a/packages/query-core/src/removable.ts b/packages/query-core/src/removable.ts index bf353266ca..69b86790aa 100644 --- a/packages/query-core/src/removable.ts +++ b/packages/query-core/src/removable.ts @@ -11,7 +11,7 @@ export abstract class Removable { protected scheduleGc(): void { this.clearGcTimeout() - if (isValidTimeout(this.gcTime)) { + if (isValidTimeout(this.gcTime) || this.gcTime === 0) { this.#gcTimeout = setTimeout(() => { this.optionalRemove() }, this.gcTime) diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index 763fdd7347..49992be78b 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -81,7 +81,7 @@ export function functionalUpdate( } export function isValidTimeout(value: unknown): value is number { - return typeof value === 'number' && value >= 0 && value !== Infinity + return typeof value === 'number' && value > 0 && value !== Infinity } export function timeUntilStale(updatedAt: number, staleTime: number): number { From 8e31ddd829fe8b8a4d96325692cac5cf9b97cc39 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:31:05 +0100 Subject: [PATCH 06/25] fix: adjust test options on query level have never been merged with previous options, so not passing staleTime now makes things stale (0) --- packages/query-core/src/__tests__/queryObserver.test.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/query-core/src/__tests__/queryObserver.test.tsx b/packages/query-core/src/__tests__/queryObserver.test.tsx index 183407787e..8927c0edb4 100644 --- a/packages/query-core/src/__tests__/queryObserver.test.tsx +++ b/packages/query-core/src/__tests__/queryObserver.test.tsx @@ -663,12 +663,12 @@ describe('queryObserver', () => { results.push(x) }) observer.setOptions({ queryKey: key, enabled: false, staleTime: 10 }) - await queryClient.fetchQuery({ queryKey: key, queryFn }) + await queryClient.fetchQuery({ queryKey: key, queryFn, staleTime: 1000 }) await sleep(20) unsubscribe() expect(queryFn).toHaveBeenCalledTimes(1) expect(results.length).toBe(2) - expect(results[0]).toMatchObject({ isStale: false, data: undefined }) + expect(results[0]).toMatchObject({ isStale: true, data: undefined }) expect(results[1]).toMatchObject({ isStale: false, data: 'data' }) }) @@ -1102,7 +1102,7 @@ describe('queryObserver', () => { unsubscribe() }) - test('disabled observers should not be stale', async () => { + test('disabled observers should also be stale', async () => { const key = queryKey() const observer = new QueryObserver(queryClient, { @@ -1111,7 +1111,7 @@ describe('queryObserver', () => { }) const result = observer.getCurrentResult() - expect(result.isStale).toBe(false) + expect(result.isStale).toBe(true) }) test('should allow staleTime as a function', async () => { From 54198b2a31bca6eceb5ce2e95c205d7b79fcc4db Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:32:43 +0100 Subject: [PATCH 07/25] chore: leftover --- packages/query-core/src/query.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 5b12535700..0099673744 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -649,8 +649,6 @@ export class Query< #updateStaleTimeout(staleTime: number): void { this.#clearStaleTimeout() - console.log('setting stale timer', staleTime) - if (isServer || this.isStale() || !isValidTimeout(staleTime)) { return } From e773cd92252449a3fceb90aa0166988dfab36a0f Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:41:05 +0100 Subject: [PATCH 08/25] fix: un-set invalidated state if we get a new staleTime given that a query is currently stale, a new staleTime that is > 0 should set it back to being not-stale --- packages/query-core/src/__tests__/queryClient.test.tsx | 3 ++- packages/query-core/src/query.ts | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/query-core/src/__tests__/queryClient.test.tsx b/packages/query-core/src/__tests__/queryClient.test.tsx index 0ad7baed30..2632008cdf 100644 --- a/packages/query-core/src/__tests__/queryClient.test.tsx +++ b/packages/query-core/src/__tests__/queryClient.test.tsx @@ -655,7 +655,7 @@ describe('queryClient', () => { const key = queryKey() queryClient.setQueryData(key, 'og') - const fetchFn = () => Promise.resolve('new') + const fetchFn = vi.fn().mockImplementation(() => Promise.resolve('new')) const first = await queryClient.fetchQuery({ queryKey: key, queryFn: fetchFn, @@ -663,6 +663,7 @@ describe('queryClient', () => { staleTime: 100, }) expect(first).toBe('og') + expect(fetchFn).toHaveBeenCalledTimes(0) }) test('should only fetch if the data is older then the given stale time', async () => { diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 0099673744..f9e0681a04 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -649,10 +649,14 @@ export class Query< #updateStaleTimeout(staleTime: number): void { this.#clearStaleTimeout() - if (isServer || this.isStale() || !isValidTimeout(staleTime)) { + if (isServer || !isValidTimeout(staleTime)) { return } + if (this.isStale() && staleTime > 0) { + this.#dispatch({ type: 'setState', state: { isInvalidated: false } }) + } + const time = timeUntilStale(this.state.dataUpdatedAt, staleTime) // The timeout is sometimes triggered 1 ms before the stale time expiration. From 27cf824703ec6a33348f3baf8e4d744163a1bd24 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 09:57:04 +0100 Subject: [PATCH 09/25] fix: also handle the case where staleTime goes from a high number to a lower one where the lower number actually makes the query instantly stale also, isStale() will always return true if we have no data yet (duh) --- .../src/__tests__/queryClient.test.tsx | 26 +++++++++++-------- packages/query-core/src/query.ts | 25 ++++++++++-------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/packages/query-core/src/__tests__/queryClient.test.tsx b/packages/query-core/src/__tests__/queryClient.test.tsx index 2632008cdf..c1753cafc6 100644 --- a/packages/query-core/src/__tests__/queryClient.test.tsx +++ b/packages/query-core/src/__tests__/queryClient.test.tsx @@ -669,36 +669,40 @@ describe('queryClient', () => { test('should only fetch if the data is older then the given stale time', async () => { const key = queryKey() - let count = 0 - const fetchFn = () => ++count + const fetchFn = vi.fn().mockImplementation(() => null) - queryClient.setQueryData(key, count) - const first = await queryClient.fetchQuery({ + queryClient.setQueryData(key, 'data') + await queryClient.fetchQuery({ queryKey: key, queryFn: fetchFn, staleTime: 100, }) + console.log(queryClient.getQueryState(key)) + // no fetch because we have fresh data (not older than 100ms) + expect(fetchFn).toHaveBeenCalledTimes(0) await sleep(11) - const second = await queryClient.fetchQuery({ + await queryClient.fetchQuery({ queryKey: key, queryFn: fetchFn, staleTime: 10, }) - const third = await queryClient.fetchQuery({ + // fetch because staleTime is now 10ms and we have waited 11ms already + expect(fetchFn).toHaveBeenCalledTimes(1) + await queryClient.fetchQuery({ queryKey: key, queryFn: fetchFn, staleTime: 10, }) + // no additional fetch because the data is still fresh + expect(fetchFn).toHaveBeenCalledTimes(1) await sleep(11) - const fourth = await queryClient.fetchQuery({ + await queryClient.fetchQuery({ queryKey: key, queryFn: fetchFn, staleTime: 10, }) - expect(first).toBe(0) - expect(second).toBe(1) - expect(third).toBe(1) - expect(fourth).toBe(2) + // another fetch after another sleep because the data is now stale + expect(fetchFn).toHaveBeenCalledTimes(2) }) }) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index f9e0681a04..d7e5e5b17e 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -295,7 +295,7 @@ export class Query< } isStale(): boolean { - return this.state.isInvalidated + return this.state.isInvalidated || this.state.data === undefined } onFocus(): void { @@ -653,19 +653,22 @@ export class Query< return } - if (this.isStale() && staleTime > 0) { - this.#dispatch({ type: 'setState', state: { isInvalidated: false } }) - } - const time = timeUntilStale(this.state.dataUpdatedAt, staleTime) - // The timeout is sometimes triggered 1 ms before the stale time expiration. - // To mitigate this issue we always add 1 ms to the timeout. - const timeout = time + 1 + const newInvalidated = time === 0 + if (this.state.isInvalidated !== newInvalidated) { + this.#dispatch({ type: 'setState', state: { isInvalidated: time === 0 } }) + } - this.#staleTimeoutId = setTimeout(() => { - this.invalidate() - }, timeout) + if (time > 0) { + // The timeout is sometimes triggered 1 ms before the stale time expiration. + // To mitigate this issue we always add 1 ms to the timeout. + const timeout = time + 1 + + this.#staleTimeoutId = setTimeout(() => { + this.invalidate() + }, timeout) + } } #clearStaleTimeout(): void { From 39b81d4da891c05bc9bedc93b9251f8d4c801811 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 10:08:13 +0100 Subject: [PATCH 10/25] fix: staleTime:Infinity should also un-set `isInvalidated` to achieve that, we move the "early return" until after we've dispatched; all the logic in between also works with staleTime: Infinity - timeUntilStale just returns Infinity as well (added tests for that, too) --- .../query-core/src/__tests__/utils.test.tsx | 10 ++++++++++ packages/query-core/src/query.ts | 20 ++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/query-core/src/__tests__/utils.test.tsx b/packages/query-core/src/__tests__/utils.test.tsx index e5ebffe3d2..7ce6cc0fad 100644 --- a/packages/query-core/src/__tests__/utils.test.tsx +++ b/packages/query-core/src/__tests__/utils.test.tsx @@ -8,6 +8,7 @@ import { partialMatchKey, replaceEqualDeep, shallowEqualObjects, + timeUntilStale, } from '../utils' import { Mutation } from '../mutation' import { createQueryClient } from './utils' @@ -30,6 +31,15 @@ describe('core/utils', () => { expect(shallowEqualObjects({ a: 1 }, undefined)).toEqual(false) }) }) + describe('timeUntilStale', () => { + it('should work with Infinity', () => { + expect(timeUntilStale(25, Infinity)).toBe(Infinity) + }) + it('should never go below zero', () => { + expect(timeUntilStale(25, -1)).toBe(0) + expect(timeUntilStale(25, -Infinity)).toBe(0) + }) + }) describe('isPlainObject', () => { it('should return `true` for a plain object', () => { expect(isPlainObject({})).toEqual(true) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index d7e5e5b17e..ad48b950f5 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -649,10 +649,6 @@ export class Query< #updateStaleTimeout(staleTime: number): void { this.#clearStaleTimeout() - if (isServer || !isValidTimeout(staleTime)) { - return - } - const time = timeUntilStale(this.state.dataUpdatedAt, staleTime) const newInvalidated = time === 0 @@ -660,15 +656,15 @@ export class Query< this.#dispatch({ type: 'setState', state: { isInvalidated: time === 0 } }) } - if (time > 0) { - // The timeout is sometimes triggered 1 ms before the stale time expiration. - // To mitigate this issue we always add 1 ms to the timeout. - const timeout = time + 1 - - this.#staleTimeoutId = setTimeout(() => { - this.invalidate() - }, timeout) + if (isServer || !isValidTimeout(staleTime) || time < 1) { + return } + + // The timeout is sometimes triggered 1 ms before the stale time expiration. + // To mitigate this issue we always add 1 ms to the timeout. + this.#staleTimeoutId = setTimeout(() => { + this.invalidate() + }, time + 1) } #clearStaleTimeout(): void { From 6c629ce4ee013a02af1205bf36cdcdcc279cb4a1 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 11:12:28 +0100 Subject: [PATCH 11/25] chore: remove logs --- packages/query-core/src/__tests__/query.test.tsx | 1 - packages/query-core/src/__tests__/queryClient.test.tsx | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 3efbc56aff..a99895ec60 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -143,7 +143,6 @@ describe('query', () => { // Promise should eventually be resolved await promise - console.log('has finished') expect(result).toBe('data3') onlineMock.mockRestore() }) diff --git a/packages/query-core/src/__tests__/queryClient.test.tsx b/packages/query-core/src/__tests__/queryClient.test.tsx index c1753cafc6..b17678454a 100644 --- a/packages/query-core/src/__tests__/queryClient.test.tsx +++ b/packages/query-core/src/__tests__/queryClient.test.tsx @@ -677,7 +677,6 @@ describe('queryClient', () => { queryFn: fetchFn, staleTime: 100, }) - console.log(queryClient.getQueryState(key)) // no fetch because we have fresh data (not older than 100ms) expect(fetchFn).toHaveBeenCalledTimes(0) await sleep(11) From a57bc552be6efc90ab1a6e2f13fab9467925610f Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 11:20:21 +0100 Subject: [PATCH 12/25] fix: "silence" update events that happen before queries were added to the cache --- packages/query-core/src/query.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index ad48b950f5..a8c1f41353 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -636,7 +636,9 @@ export class Query< observer.onQueryUpdate() }) - this.#cache.notify({ query: this, type: 'updated', action }) + if (this.#cache.get(this.queryHash)) { + this.#cache.notify({ query: this, type: 'updated', action }) + } }) } From 30735d9b701c54eec372ad21117ce301b0c67470 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 11:20:37 +0100 Subject: [PATCH 13/25] fix: there's now an extra update event on the cache (expected) --- packages/query-core/src/__tests__/queryCache.test.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/queryCache.test.tsx b/packages/query-core/src/__tests__/queryCache.test.tsx index 8f8106b0ff..4d3f499786 100644 --- a/packages/query-core/src/__tests__/queryCache.test.tsx +++ b/packages/query-core/src/__tests__/queryCache.test.tsx @@ -55,7 +55,7 @@ describe('queryCache', () => { const unsubScribeObserver = observer.subscribe(vi.fn()) await waitFor(() => { - expect(events.length).toBe(8) + expect(events.length).toBe(9) }) expect(events).toEqual([ @@ -67,6 +67,7 @@ describe('queryCache', () => { 'observerResultsUpdated', // 6. Observer result updated -> success 'updated', // 7. Query updated -> success 'observerResultsUpdated', // 8. Observer result updated -> stale + 'updated', // 9. Query updated -> stale ]) queries.forEach((query) => { From 2747f7e18c5d20e41b006df56c196a9ed310c26a Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 11:20:54 +0100 Subject: [PATCH 14/25] test: queries are now invalidated with staleTime 0 (expected) --- packages/query-core/src/__tests__/hydration.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/query-core/src/__tests__/hydration.test.tsx b/packages/query-core/src/__tests__/hydration.test.tsx index f3ebc6969c..f37a7e52f5 100644 --- a/packages/query-core/src/__tests__/hydration.test.tsx +++ b/packages/query-core/src/__tests__/hydration.test.tsx @@ -891,7 +891,7 @@ describe('dehydration and rehydration', () => { fetchFailureReason: null, fetchMeta: null, fetchStatus: 'fetching', - isInvalidated: false, + isInvalidated: true, status: 'pending', }, ) @@ -910,7 +910,7 @@ describe('dehydration and rehydration', () => { fetchFailureReason: null, fetchMeta: null, fetchStatus: 'idle', - isInvalidated: false, + isInvalidated: true, status: 'success', }), ) From 08920a9fc3b8b9b07ec6be899fecf388ee0f2802 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 11:26:01 +0100 Subject: [PATCH 15/25] test: queries without staleTime are now instantly invalidated (expected) --- .../query-core/src/__tests__/queryCache.test.tsx | 2 ++ .../query-core/src/__tests__/queryClient.test.tsx | 14 +++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/queryCache.test.tsx b/packages/query-core/src/__tests__/queryCache.test.tsx index 4d3f499786..dd8c0bd257 100644 --- a/packages/query-core/src/__tests__/queryCache.test.tsx +++ b/packages/query-core/src/__tests__/queryCache.test.tsx @@ -166,6 +166,7 @@ describe('queryCache', () => { await queryClient.prefetchQuery({ queryKey: key1, queryFn: () => 'data1', + staleTime: 100, }) await queryClient.prefetchQuery({ queryKey: key2, @@ -174,6 +175,7 @@ describe('queryCache', () => { await queryClient.prefetchQuery({ queryKey: [{ a: 'a', b: 'b' }], queryFn: () => 'data3', + staleTime: 100, }) await queryClient.prefetchQuery({ queryKey: ['posts', 1], diff --git a/packages/query-core/src/__tests__/queryClient.test.tsx b/packages/query-core/src/__tests__/queryClient.test.tsx index b17678454a..65d6530720 100644 --- a/packages/query-core/src/__tests__/queryClient.test.tsx +++ b/packages/query-core/src/__tests__/queryClient.test.tsx @@ -1094,14 +1094,23 @@ describe('queryClient', () => { test('should be able to refetch all stale queries', async () => { const key1 = queryKey() const key2 = queryKey() + const key3 = queryKey() const queryFn1 = vi .fn<(...args: Array) => string>() .mockReturnValue('data1') const queryFn2 = vi .fn<(...args: Array) => string>() .mockReturnValue('data2') + const queryFn3 = vi + .fn<(...args: Array) => string>() + .mockReturnValue('data3') await queryClient.fetchQuery({ queryKey: key1, queryFn: queryFn1 }) await queryClient.fetchQuery({ queryKey: key2, queryFn: queryFn2 }) + await queryClient.fetchQuery({ + queryKey: key3, + queryFn: queryFn3, + staleTime: 1, + }) const observer = new QueryObserver(queryClient, { queryKey: key1, queryFn: queryFn1, @@ -1112,7 +1121,10 @@ describe('queryClient', () => { unsubscribe() // fetchQuery, observer mount, invalidation (cancels observer mount) and refetch expect(queryFn1).toHaveBeenCalledTimes(4) - expect(queryFn2).toHaveBeenCalledTimes(1) + // fetchQuery, refetch + expect(queryFn2).toHaveBeenCalledTimes(2) + // fetchQuery + expect(queryFn3).toHaveBeenCalledTimes(1) }) test('should be able to refetch all stale and active queries', async () => { From 92a109db36f98f8586d8618c6e77ddbdeca6af69 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 18 Nov 2024 11:43:42 +0100 Subject: [PATCH 16/25] fix: do not call setOptions when getting from `build` setOptions doesn't merge, so we need to call it at the specific places where we want it to overwrite - fetchQuery and ensureQueryData --- packages/query-core/src/queryCache.ts | 2 -- packages/query-core/src/queryClient.ts | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/query-core/src/queryCache.ts b/packages/query-core/src/queryCache.ts index 16b6c83c20..3c3b25a103 100644 --- a/packages/query-core/src/queryCache.ts +++ b/packages/query-core/src/queryCache.ts @@ -126,8 +126,6 @@ export class QueryCache extends Subscribable { defaultOptions: client.getQueryDefaults(queryKey), }) this.add(query) - } else { - query.setOptions(defaultedOptions) } return query diff --git a/packages/query-core/src/queryClient.ts b/packages/query-core/src/queryClient.ts index d342f39934..c9063b003c 100644 --- a/packages/query-core/src/queryClient.ts +++ b/packages/query-core/src/queryClient.ts @@ -142,6 +142,7 @@ export class QueryClient { else { const defaultedOptions = this.defaultQueryOptions(options) const query = this.#queryCache.build(this, defaultedOptions) + query.setOptions(defaultedOptions) if (options.revalidateIfStale && query.isStale()) { void this.prefetchQuery(defaultedOptions) @@ -342,6 +343,7 @@ export class QueryClient { } const query = this.#queryCache.build(this, defaultedOptions) + query.setOptions(defaultedOptions) return query.isStale() ? query.fetch(defaultedOptions) From f91fb77e108ea0349c90717ff12be2cf51a78d18 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 09:37:25 +0100 Subject: [PATCH 17/25] fix: queries without data are always stale --- packages/react-query/src/__tests__/useQuery.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index e336b99815..4e6bd98f5b 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -1254,7 +1254,7 @@ describe('useQuery', () => { data: undefined, isFetching: false, isSuccess: false, - isStale: false, + isStale: true, }) }) @@ -1294,7 +1294,7 @@ describe('useQuery', () => { data: undefined, isFetching: false, isSuccess: false, - isStale: false, + isStale: true, }) }) From c3230173707b3e2c8e7ad16bf66aa450a3b66c29 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 09:41:20 +0100 Subject: [PATCH 18/25] test: queries without data are always stale, even if the observer is disabled --- packages/react-query/src/__tests__/useQuery.test.tsx | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index 4e6bd98f5b..e23f8f0a68 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -3985,8 +3985,7 @@ describe('useQuery', () => { expect(results.length).toBe(3) expect(results[0]).toMatchObject({ data: 'initial', isStale: true }) expect(results[1]).toMatchObject({ data: 'fetched data', isStale: true }) - // disabled observers are not stale - expect(results[2]).toMatchObject({ data: 'fetched data', isStale: false }) + expect(results[2]).toMatchObject({ data: 'fetched data', isStale: true }) }) it('should support enabled:false in query object syntax', async () => { @@ -4910,14 +4909,14 @@ describe('useQuery', () => { isPending: true, isFetching: false, isSuccess: false, - isStale: false, + isStale: true, }) expect(states[1]).toMatchObject({ data: undefined, isPending: true, isFetching: true, isSuccess: false, - isStale: false, + isStale: true, }) expect(states[2]).toMatchObject({ data: 1, @@ -4931,7 +4930,7 @@ describe('useQuery', () => { isPending: true, isFetching: false, isSuccess: false, - isStale: false, + isStale: true, }) }) From 4cc1d9bf7473adcc12cf724ebd83e4b394c70751 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 09:46:13 +0100 Subject: [PATCH 19/25] refactor: re-use variable --- packages/query-core/src/query.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index a8c1f41353..b2c1d5d8d0 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -655,7 +655,10 @@ export class Query< const newInvalidated = time === 0 if (this.state.isInvalidated !== newInvalidated) { - this.#dispatch({ type: 'setState', state: { isInvalidated: time === 0 } }) + this.#dispatch({ + type: 'setState', + state: { isInvalidated: newInvalidated }, + }) } if (isServer || !isValidTimeout(staleTime) || time < 1) { From b07b3faf2a832e5650f43774226d731738c9fab3 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 09:58:44 +0100 Subject: [PATCH 20/25] chore: remove a test that makes no sense anymore --- .../src/__tests__/useQuery.test.tsx | 101 ------------------ 1 file changed, 101 deletions(-) diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index e23f8f0a68..7999ab8693 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -2040,107 +2040,6 @@ describe('useQuery', () => { }) }) - it('should be able to set different stale times for a query', async () => { - const key = queryKey() - const states1: Array> = [] - const states2: Array> = [] - - await queryClient.prefetchQuery({ - queryKey: key, - queryFn: async () => { - await sleep(10) - return 'prefetch' - }, - }) - - await sleep(20) - - function FirstComponent() { - const state = useQuery({ - queryKey: key, - queryFn: async () => { - await sleep(10) - return 'one' - }, - - staleTime: 100, - }) - states1.push(state) - return null - } - - function SecondComponent() { - const state = useQuery({ - queryKey: key, - queryFn: async () => { - await sleep(10) - return 'two' - }, - - staleTime: 10, - }) - states2.push(state) - return null - } - - function Page() { - return ( - <> - - - - ) - } - - renderWithClient(queryClient, ) - - await sleep(200) - - expect(states1.length).toBe(4) - expect(states2.length).toBe(3) - - expect(states1).toMatchObject([ - // First render - { - data: 'prefetch', - isStale: false, - }, - // Second useQuery started fetching - { - data: 'prefetch', - isStale: false, - }, - // Second useQuery data came in - { - data: 'two', - isStale: false, - }, - // Data became stale after 100ms - { - data: 'two', - isStale: true, - }, - ]) - - expect(states2).toMatchObject([ - // First render, data is stale and starts fetching - { - data: 'prefetch', - isStale: true, - }, - // Second useQuery data came in - { - data: 'two', - isStale: false, - }, - // Data became stale after 5ms - { - data: 'two', - isStale: true, - }, - ]) - }) - it('should re-render when a query becomes stale', async () => { const key = queryKey() const states: Array> = [] From 057b59e29aaa95e273cce2bd3277fa41ec9ea6e7 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 10:48:17 +0100 Subject: [PATCH 21/25] refactor(query): make sure to always update staleTimeout in the constructor that way, this.state.isInvalidated is set correctly for staleTime: 0 --- packages/query-core/src/query.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index b2c1d5d8d0..750ba17ceb 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -194,9 +194,9 @@ export class Query< const nextStaleTime = this.#resolveStaleTime(this.options.staleTime) if (nextStaleTime === 0) { this.#initialState.isInvalidated = true - } else { - this.#updateStaleTimeout(nextStaleTime) } + + this.#updateStaleTimeout(nextStaleTime) } get meta(): QueryMeta | undefined { return this.options.meta @@ -661,7 +661,7 @@ export class Query< }) } - if (isServer || !isValidTimeout(staleTime) || time < 1) { + if (isServer || !isValidTimeout(staleTime) || newInvalidated) { return } @@ -732,7 +732,7 @@ function getDefaultState< fetchFailureCount: 0, fetchFailureReason: null, fetchMeta: null, - isInvalidated: false, + isInvalidated: !hasData, status: hasData ? 'success' : 'pending', fetchStatus: 'idle', } From 8c1c028f4eb8d2ec1f7fe50c949d0ca782da7d74 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 10:56:08 +0100 Subject: [PATCH 22/25] chore: inline leftover --- packages/query-core/src/queryCache.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/query-core/src/queryCache.ts b/packages/query-core/src/queryCache.ts index 3c3b25a103..8f637e0a39 100644 --- a/packages/query-core/src/queryCache.ts +++ b/packages/query-core/src/queryCache.ts @@ -114,14 +114,13 @@ export class QueryCache extends Subscribable { const queryHash = options.queryHash ?? hashQueryKeyByOptions(queryKey, options) let query = this.get(queryHash) - const defaultedOptions = client.defaultQueryOptions(options) if (!query) { query = new Query({ cache: this, queryKey, queryHash, - options: defaultedOptions, + options: client.defaultQueryOptions(options), state, defaultOptions: client.getQueryDefaults(queryKey), }) From 3ae6bff73159bce8677d281789df638022c301a6 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 20 Nov 2024 15:28:21 +0100 Subject: [PATCH 23/25] feat: warn when there are multiple observers mounted at the same time with different staleTimes --- .../query-core/src/__tests__/query.test.tsx | 56 +++++++++++++++++++ packages/query-core/src/query.ts | 13 +++++ 2 files changed, 69 insertions(+) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index a99895ec60..f5901e9501 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -1045,4 +1045,60 @@ describe('query', () => { expect(query.state.status).toBe('error') }) + + it('should warn when multiple observers have different staleTimes when mounted at the same time', async () => { + const consoleMock = vi.spyOn(console, 'error') + consoleMock.mockImplementation(() => undefined) + const key = queryKey() + const observer1 = new QueryObserver(queryClient, { + queryKey: key, + initialData: 5, + staleTime: 1000, + }) + + const unsubscribe1 = observer1.subscribe(() => undefined) + + const observer2 = new QueryObserver(queryClient, { + queryKey: key, + initialData: 5, + staleTime: 2000, + }) + + const unsubscribe2 = observer2.subscribe(() => undefined) + + expect(consoleMock).toHaveBeenCalledTimes(1) + expect(consoleMock).toHaveBeenCalledWith( + `Multiple observers with different staleTimes are attached to the this query: ["${key}"]. The staleTime of the query will be determined by the last observer.`, + ) + unsubscribe1() + unsubscribe2() + consoleMock.mockRestore() + }) + + it('should not warn when multiple observers have different staleTimes but not mounted at the same time', async () => { + const consoleMock = vi.spyOn(console, 'error') + consoleMock.mockImplementation(() => undefined) + const key = queryKey() + const observer1 = new QueryObserver(queryClient, { + queryKey: key, + initialData: 5, + staleTime: 1000, + }) + + const unsubscribe1 = observer1.subscribe(() => undefined) + + unsubscribe1() + + const observer2 = new QueryObserver(queryClient, { + queryKey: key, + initialData: 5, + staleTime: 2000, + }) + + const unsubscribe2 = observer2.subscribe(() => undefined) + + expect(consoleMock).toHaveBeenCalledTimes(0) + unsubscribe2() + consoleMock.mockRestore() + }) }) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index 750ba17ceb..dd27d5cd00 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -653,6 +653,19 @@ export class Query< const time = timeUntilStale(this.state.dataUpdatedAt, staleTime) + if (process.env.NODE_ENV !== 'production') { + const staleTimes = this.observers.map((observer) => + this.#resolveStaleTime(observer.options.staleTime), + ) + staleTimes.push(staleTime) + + if (new Set(staleTimes).size > 1) { + console.error( + `Multiple observers with different staleTimes are attached to the this query: ${this.queryHash}. The staleTime of the query will be determined by the last observer.`, + ) + } + } + const newInvalidated = time === 0 if (this.state.isInvalidated !== newInvalidated) { this.#dispatch({ From 33058d1af6a09e578e95341ad5660602adf7b03c Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 22 Nov 2024 11:10:59 +0100 Subject: [PATCH 24/25] fix(types): There's no need to define staleTime additionally on FetchQueryOptions, because staleTime is now an option on the query anyway --- packages/query-core/src/types.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/query-core/src/types.ts b/packages/query-core/src/types.ts index a903d52f7e..72a5d69e62 100644 --- a/packages/query-core/src/types.ts +++ b/packages/query-core/src/types.ts @@ -454,11 +454,6 @@ export interface FetchQueryOptions< 'queryKey' > { initialPageParam?: never - /** - * The time in milliseconds after data is considered stale. - * If the data is fresh it will be returned from the cache. - */ - staleTime?: StaleTime } export interface EnsureQueryDataOptions< From 6529df283d3a96d3e225e045787a66199699c9a8 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 22 Nov 2024 11:14:39 +0100 Subject: [PATCH 25/25] test: add another case --- .../src/__tests__/queryClient.test.tsx | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/query-core/src/__tests__/queryClient.test.tsx b/packages/query-core/src/__tests__/queryClient.test.tsx index 65d6530720..3d40d69bc4 100644 --- a/packages/query-core/src/__tests__/queryClient.test.tsx +++ b/packages/query-core/src/__tests__/queryClient.test.tsx @@ -703,6 +703,38 @@ describe('queryClient', () => { // another fetch after another sleep because the data is now stale expect(fetchFn).toHaveBeenCalledTimes(2) }) + + test('should fetch if data has been manually invalidated', async () => { + const key = queryKey() + + const fetchFn = vi.fn().mockImplementation(() => null) + + queryClient.setQueryDefaults(key, { staleTime: Infinity }) + queryClient.setQueryData(key, 'data') + + await queryClient.fetchQuery({ + queryKey: key, + queryFn: fetchFn, + staleTime: 100, + }) + // no fetch because we have fresh data (not older than 100ms) + expect(fetchFn).toHaveBeenCalledTimes(0) + + // mark query as invalidated + await queryClient.invalidateQueries({ + queryKey: key, + refetchType: 'none', + }) + + await queryClient.fetchQuery({ + queryKey: key, + queryFn: fetchFn, + staleTime: 100, + }) + + // fetch because even if not stale by time, it's manually invalidated + expect(fetchFn).toHaveBeenCalledTimes(1) + }) }) describe('fetchInfiniteQuery', () => {