From 355e86695dbe66c2fc8c485d3d7ed1563c1854a2 Mon Sep 17 00:00:00 2001 From: Niek Date: Thu, 27 Aug 2020 15:34:10 +0200 Subject: [PATCH] feat: determine staleness locally instead of globally --- src/core/query.ts | 91 ++++++----------------------- src/core/queryCache.ts | 4 +- src/core/queryObserver.ts | 92 +++++++++++++++++++++++++++--- src/core/tests/queryCache.test.tsx | 79 +++++++++---------------- src/react/tests/useQuery.test.tsx | 88 +++++++++++++++++++++++++++- 5 files changed, 218 insertions(+), 136 deletions(-) diff --git a/src/core/query.ts b/src/core/query.ts index 3de938ad9b5..7d8ae221be6 100644 --- a/src/core/query.ts +++ b/src/core/query.ts @@ -44,7 +44,6 @@ export interface QueryState { isFetchingMore: IsFetchingMoreValue isIdle: boolean isLoading: boolean - isStale: boolean isSuccess: boolean status: QueryStatus throwInErrorBoundary?: boolean @@ -66,7 +65,6 @@ export interface RefetchOptions { export enum ActionType { Failed = 'Failed', - MarkStale = 'MarkStale', Fetch = 'Fetch', Success = 'Success', Error = 'Error', @@ -76,10 +74,6 @@ interface FailedAction { type: ActionType.Failed } -interface MarkStaleAction { - type: ActionType.MarkStale -} - interface FetchAction { type: ActionType.Fetch isFetchingMore?: IsFetchingMoreValue @@ -89,7 +83,6 @@ interface SuccessAction { type: ActionType.Success data: TResult | undefined canFetchMore?: boolean - isStale: boolean } interface ErrorAction { @@ -101,7 +94,6 @@ export type Action = | ErrorAction | FailedAction | FetchAction - | MarkStaleAction | SuccessAction // CLASS @@ -136,7 +128,6 @@ export class Query { activateTimeouts(): void { this.enableTimeouts = true - this.rescheduleStaleTimeout() this.rescheduleGarbageCollection() } @@ -150,45 +141,6 @@ export class Query { this.notifyGlobalListeners(this) } - private rescheduleStaleTimeout(): void { - if (isServer) { - return - } - - this.clearStaleTimeout() - - if ( - !this.enableTimeouts || - this.state.isStale || - this.state.status !== QueryStatus.Success || - this.config.staleTime === Infinity - ) { - return - } - - const staleTime = this.config.staleTime || 0 - let timeout = staleTime - if (this.state.updatedAt) { - const timeElapsed = Date.now() - this.state.updatedAt - const timeUntilStale = staleTime - timeElapsed - timeout = Math.max(timeUntilStale, 0) - } - - this.staleTimeout = setTimeout(() => { - this.invalidate() - }, timeout) - } - - invalidate(): void { - this.clearStaleTimeout() - - if (this.state.isStale) { - return - } - - this.dispatch({ type: ActionType.MarkStale }) - } - private rescheduleGarbageCollection(): void { if (isServer) { return @@ -230,7 +182,7 @@ export class Query { private clearTimersObservers(): void { this.observers.forEach(observer => { - observer.clearRefetchInterval() + observer.clearTimers() }) } @@ -264,8 +216,6 @@ export class Query { data = prevData } - const isStale = this.config.staleTime === 0 - // Try to determine if more data can be fetched const canFetchMore = hasMorePages(this.config, data) @@ -273,11 +223,8 @@ export class Query { this.dispatch({ type: ActionType.Success, data, - isStale, canFetchMore, }) - - this.rescheduleStaleTimeout() } clear(): void { @@ -293,12 +240,23 @@ export class Query { return this.observers.some(observer => observer.config.enabled) } + isStale(): boolean { + return this.observers.some(observer => observer.isStale()) + } + + isStaleByTime(staleTime = 0): boolean { + return ( + !this.state.isSuccess || this.state.updatedAt + staleTime <= Date.now() + ) + } + onWindowFocus(): void { if ( - this.state.isStale && this.observers.some( observer => - observer.config.enabled && observer.config.refetchOnWindowFocus + observer.isStale() && + observer.config.enabled && + observer.config.refetchOnWindowFocus ) ) { this.fetch() @@ -308,10 +266,11 @@ export class Query { onOnline(): void { if ( - this.state.isStale && this.observers.some( observer => - observer.config.enabled && observer.config.refetchOnReconnect + observer.isStale() && + observer.config.enabled && + observer.config.refetchOnReconnect ) ) { this.fetch() @@ -628,12 +587,6 @@ function getDefaultState( const hasInitialData = typeof initialData !== 'undefined' - const isStale = - !config.enabled || - (typeof config.initialStale === 'function' - ? config.initialStale() - : config.initialStale ?? !hasInitialData) - const initialStatus = hasInitialData ? QueryStatus.Success : config.enabled @@ -647,9 +600,8 @@ function getDefaultState( isFetching: initialStatus === QueryStatus.Loading, isFetchingMore: false, failureCount: 0, - isStale, data: initialData, - updatedAt: hasInitialData ? Date.now() : 0, + updatedAt: Date.now(), canFetchMore: hasMorePages(config, initialData), } } @@ -664,11 +616,6 @@ export function queryReducer( ...state, failureCount: state.failureCount + 1, } - case ActionType.MarkStale: - return { - ...state, - isStale: true, - } case ActionType.Fetch: const status = typeof state.data !== 'undefined' @@ -687,7 +634,6 @@ export function queryReducer( ...getStatusProps(QueryStatus.Success), data: action.data, error: null, - isStale: action.isStale, isFetched: true, isFetching: false, isFetchingMore: false, @@ -703,7 +649,6 @@ export function queryReducer( isFetched: true, isFetching: false, isFetchingMore: false, - isStale: true, failureCount: state.failureCount + 1, throwInErrorBoundary: true, } diff --git a/src/core/queryCache.ts b/src/core/queryCache.ts index d6ee1eac60e..1448e73cac1 100644 --- a/src/core/queryCache.ts +++ b/src/core/queryCache.ts @@ -189,7 +189,7 @@ export class QueryCache { } } - return query.invalidate() + return undefined }) ) } catch (err) { @@ -305,7 +305,7 @@ export class QueryCache { let query try { query = this.buildQuery(queryKey, configWithoutRetry) - if (options?.force || query.state.isStale) { + if (options?.force || query.isStaleByTime(config.staleTime)) { await query.fetch() } return query.state.data diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts index bed80161a24..3d3f3e41636 100644 --- a/src/core/queryObserver.ts +++ b/src/core/queryObserver.ts @@ -19,6 +19,7 @@ export class QueryObserver { private currentResult!: QueryResult private previousResult?: QueryResult private updateListener?: UpdateListener + private staleTimeoutId?: number private refetchIntervalId?: number private started?: boolean @@ -39,14 +40,14 @@ export class QueryObserver { this.updateListener = listener this.currentQuery.subscribeObserver(this) this.optionalFetch() - this.updateRefetchInterval() + this.updateTimers() return this.unsubscribe.bind(this) } unsubscribe(): void { this.started = false this.updateListener = undefined - this.clearRefetchInterval() + this.clearTimers() this.currentQuery.unsubscribeObserver(this) } @@ -64,7 +65,7 @@ export class QueryObserver { // If we subscribed to a new query, optionally fetch and update refetch if (updated) { this.optionalFetch() - this.updateRefetchInterval() + this.updateTimers() return } @@ -73,6 +74,14 @@ export class QueryObserver { this.optionalFetch() } + // Update stale interval if needed + if ( + config.enabled !== prevConfig.enabled || + config.staleTime !== prevConfig.staleTime + ) { + this.updateStaleTimeout() + } + // Update refetch interval if needed if ( config.enabled !== prevConfig.enabled || @@ -84,6 +93,10 @@ export class QueryObserver { } } + isStale(): boolean { + return this.currentResult.isStale + } + getCurrentResult(): QueryResult { return this.currentResult } @@ -125,6 +138,37 @@ export class QueryObserver { } } + private updateIsStale(): void { + const isStale = this.currentQuery.isStaleByTime(this.config.staleTime) + if (isStale !== this.currentResult.isStale) { + this.currentResult = this.createResult() + this.updateListener?.(this.currentResult) + } + } + + private updateStaleTimeout(): void { + if (isServer) { + return + } + + this.clearStaleTimeout() + + const staleTime = this.config.staleTime || 0 + const { isStale, updatedAt } = this.currentResult + + if (isStale || staleTime === Infinity) { + return + } + + const timeElapsed = Date.now() - updatedAt + const timeUntilStale = staleTime - timeElapsed + const timeout = Math.max(timeUntilStale, 0) + + this.staleTimeoutId = setTimeout(() => { + this.updateIsStale() + }, timeout) + } + private updateRefetchInterval(): void { if (isServer) { return @@ -148,7 +192,24 @@ export class QueryObserver { }, this.config.refetchInterval) } - clearRefetchInterval(): void { + updateTimers(): void { + this.updateStaleTimeout() + this.updateRefetchInterval() + } + + clearTimers(): void { + this.clearStaleTimeout() + this.clearRefetchInterval() + } + + private clearStaleTimeout(): void { + if (this.staleTimeoutId) { + clearInterval(this.staleTimeoutId) + this.staleTimeoutId = undefined + } + } + + private clearRefetchInterval(): void { if (this.refetchIntervalId) { clearInterval(this.refetchIntervalId) this.refetchIntervalId = undefined @@ -156,7 +217,7 @@ export class QueryObserver { } private createResult(): QueryResult { - const { currentQuery, previousResult, config } = this + const { currentResult, currentQuery, previousResult, config } = this const { canFetchMore, @@ -166,7 +227,6 @@ export class QueryObserver { isFetching, isFetchingMore, isLoading, - isStale, } = currentQuery.state let { data, status, updatedAt } = currentQuery.state @@ -178,6 +238,22 @@ export class QueryObserver { status = previousResult.status } + let isStale = false + + // When the query has not been fetched yet and this is the initial render, + // determine the staleness based on the initialStale or existence of initial data. + if (!currentResult && !currentQuery.state.isFetched) { + if (typeof config.initialStale === 'function') { + isStale = config.initialStale() + } else if (typeof config.initialStale === 'boolean') { + isStale = config.initialStale + } else { + isStale = typeof currentQuery.state.data === 'undefined' + } + } else { + isStale = currentQuery.isStaleByTime(config.staleTime) + } + return { ...getStatusProps(status), canFetchMore, @@ -236,11 +312,11 @@ export class QueryObserver { if (action.type === 'Success' && isSuccess) { this.config.onSuccess?.(data!) this.config.onSettled?.(data!, null) - this.updateRefetchInterval() + this.updateTimers() } else if (action.type === 'Error' && isError) { this.config.onError?.(error!) this.config.onSettled?.(undefined, error!) - this.updateRefetchInterval() + this.updateTimers() } this.updateListener?.(this.currentResult) diff --git a/src/core/tests/queryCache.test.tsx b/src/core/tests/queryCache.test.tsx index c84f3b2f29e..1a7df97addd 100644 --- a/src/core/tests/queryCache.test.tsx +++ b/src/core/tests/queryCache.test.tsx @@ -71,6 +71,7 @@ describe('queryCache', () => { fetchFn, { initialData: 'initial', + staleTime: 100, }, { throwOnError: true, @@ -79,6 +80,33 @@ describe('queryCache', () => { expect(first).toBe('og') }) + test('prefetchQuery should only fetch if the data is older then the given stale time', async () => { + const key = queryKey() + + let count = 0 + const fetchFn = () => ++count + + defaultQueryCache.setQueryData(key, count) + const first = await defaultQueryCache.prefetchQuery(key, fetchFn, { + staleTime: 100, + }) + await sleep(11) + const second = await defaultQueryCache.prefetchQuery(key, fetchFn, { + staleTime: 10, + }) + const third = await defaultQueryCache.prefetchQuery(key, fetchFn, { + staleTime: 10, + }) + await sleep(11) + const fourth = await defaultQueryCache.prefetchQuery(key, fetchFn, { + staleTime: 10, + }) + expect(first).toBe(0) + expect(second).toBe(1) + expect(third).toBe(1) + expect(fourth).toBe(2) + }) + test('prefetchQuery should throw error when throwOnError is true', async () => { const consoleMock = mockConsoleError() @@ -200,43 +228,6 @@ describe('queryCache', () => { expect(defaultQueryCache.getQuery(key)).toBeFalsy() }) - test('setQueryData should schedule stale timeout, if staleTime is set', async () => { - const key = queryKey() - - defaultQueryCache.setQueryData(key, 'test data', { staleTime: 10 }) - // @ts-expect-error - expect(defaultQueryCache.getQuery(key)!.staleTimeout).not.toBeUndefined() - }) - - test('setQueryData should not schedule stale timeout by default', async () => { - const key = queryKey() - - defaultQueryCache.setQueryData(key, 'test data') - // @ts-expect-error - expect(defaultQueryCache.getQuery(key)!.staleTimeout).toBeUndefined() - }) - - test('setQueryData should not schedule stale timeout, if staleTime is set to `Infinity`', async () => { - const key = queryKey() - - defaultQueryCache.setQueryData(key, 'test data', { staleTime: Infinity }) - // @ts-expect-error - expect(defaultQueryCache.getQuery(key)!.staleTimeout).toBeUndefined() - }) - - test('setQueryData schedules stale timeouts appropriately', async () => { - const key = queryKey() - - defaultQueryCache.setQueryData(key, 'test data', { staleTime: 100 }) - - expect(defaultQueryCache.getQuery(key)!.state.data).toEqual('test data') - expect(defaultQueryCache.getQuery(key)!.state.isStale).toEqual(false) - - await new Promise(resolve => setTimeout(resolve, 100)) - - expect(defaultQueryCache.getQuery(key)!.state.isStale).toEqual(true) - }) - test('setQueryData updater function works as expected', () => { const key = queryKey() @@ -266,20 +257,6 @@ describe('queryCache', () => { expect(data).toEqual(['data1', 'data2']) }) - test('stale timeout dispatch is not called if query is no longer in the query cache', async () => { - const key = queryKey() - - const fetchData = () => Promise.resolve('data') - await defaultQueryCache.prefetchQuery(key, fetchData, { - staleTime: 100, - }) - const query = defaultQueryCache.getQuery(key) - expect(query!.state.isStale).toBe(false) - defaultQueryCache.removeQueries(key) - await sleep(50) - expect(query!.state.isStale).toBe(false) - }) - test('query interval is cleared when unsubscribed to a refetchInterval query', async () => { const key = queryKey() diff --git a/src/react/tests/useQuery.test.tsx b/src/react/tests/useQuery.test.tsx index 6e4966eb508..2307b4efe99 100644 --- a/src/react/tests/useQuery.test.tsx +++ b/src/react/tests/useQuery.test.tsx @@ -461,6 +461,88 @@ describe('useQuery', () => { }) }) + it('should be able to set different stale times for a query', async () => { + const key = queryKey() + const states1: QueryResult[] = [] + const states2: QueryResult[] = [] + + await queryCache.prefetchQuery(key, () => 'prefetch') + + await sleep(10) + + function FirstComponent() { + const state = useQuery(key, () => 'one', { + staleTime: 100, + }) + states1.push(state) + return null + } + + function SecondComponent() { + const state = useQuery(key, () => 'two', { + staleTime: 5, + }) + states2.push(state) + return null + } + + function Page() { + return ( + <> + + + + ) + } + + render() + + await waitFor(() => expect(states1.length).toBe(4)) + await waitFor(() => expect(states2.length).toBe(4)) + + // First render + expect(states1[0]).toMatchObject({ + data: 'prefetch', + isStale: false, + }) + // Second useQuery started fetching + expect(states1[1]).toMatchObject({ + data: 'prefetch', + isStale: false, + }) + // Second useQuery data came in + expect(states1[2]).toMatchObject({ + data: 'two', + isStale: false, + }) + // Data became stale after 100ms + expect(states1[3]).toMatchObject({ + data: 'two', + isStale: true, + }) + + // First render, data is stale + expect(states2[0]).toMatchObject({ + data: 'prefetch', + isStale: true, + }) + // Second useQuery started fetching + expect(states2[1]).toMatchObject({ + data: 'prefetch', + isStale: true, + }) + // Second useQuery data came in + expect(states2[2]).toMatchObject({ + data: 'two', + isStale: false, + }) + // Data became stale after 5ms + expect(states2[3]).toMatchObject({ + data: 'two', + isStale: true, + }) + }) + // See https://github.com/tannerlinsley/react-query/issues/137 it('should not override initial data in dependent queries', async () => { const key1 = queryKey() @@ -767,6 +849,7 @@ describe('useQuery', () => { // See https://github.com/tannerlinsley/react-query/issues/195 it('should refetch if stale after a prefetch', async () => { const key = queryKey() + const states: QueryResult[] = [] const queryFn = jest.fn() queryFn.mockImplementation(() => 'data') @@ -781,13 +864,14 @@ describe('useQuery', () => { await sleep(11) function Page() { - useQuery(key, queryFn) + const state = useQuery(key, queryFn) + states.push(state) return null } render() - await act(() => sleep(0)) + await waitFor(() => expect(states.length).toBe(3)) expect(prefetchQueryFn).toHaveBeenCalledTimes(1) expect(queryFn).toHaveBeenCalledTimes(1)