From 8f731939a6cdbf20d2e4d888079234bc96ba277a Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sat, 9 Mar 2024 13:07:21 +0100 Subject: [PATCH 1/5] fix: do not notify the cache of `observerOptionsUpdated` if the prevOptions weren't defaulted non-defaulted prev options indicate a "mount" event, in which case we don't want to fire an "update" event --- packages/query-core/src/queryObserver.ts | 5 ++++- packages/query-core/src/tests/queryCache.test.tsx | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index b28dd19199..a5e59ffa16 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -155,7 +155,10 @@ export class QueryObserver< this.#updateQuery() - if (!shallowEqualObjects(this.options, prevOptions)) { + if ( + prevOptions._defaulted && + !shallowEqualObjects(this.options, prevOptions) + ) { this.#client.getQueryCache().notify({ type: 'observerOptionsUpdated', query: this.#currentQuery, diff --git a/packages/query-core/src/tests/queryCache.test.tsx b/packages/query-core/src/tests/queryCache.test.tsx index f24fb0ae0e..8f8106b0ff 100644 --- a/packages/query-core/src/tests/queryCache.test.tsx +++ b/packages/query-core/src/tests/queryCache.test.tsx @@ -55,12 +55,11 @@ describe('queryCache', () => { const unsubScribeObserver = observer.subscribe(vi.fn()) await waitFor(() => { - expect(events.length).toBe(9) + expect(events.length).toBe(8) }) expect(events).toEqual([ 'added', // 1. Query added -> loading - 'observerOptionsUpdated', 'observerResultsUpdated', // 2. Observer result updated -> loading 'observerAdded', // 3. Observer added 'observerResultsUpdated', // 4. Observer result updated -> fetching From 48ce79507b133fd29936ec5ae00e74d9c6101681 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sat, 9 Mar 2024 13:29:29 +0100 Subject: [PATCH 2/5] fix: react-query tests with the new assumption that disabled observers are not stale --- packages/query-core/src/queryObserver.ts | 1 - .../src/tests/queryObserver.test.tsx | 24 ++++++++++++++----- .../src/__tests__/useQuery.test.tsx | 21 ++++++++-------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index a5e59ffa16..4035c82064 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -731,7 +731,6 @@ function shouldFetchOptionally( prevOptions: QueryObserverOptions, ): boolean { return ( - options.enabled !== false && (query !== prevQuery || prevOptions.enabled === false) && (!options.suspense || query.state.status !== 'error') && isStale(query, options) diff --git a/packages/query-core/src/tests/queryObserver.test.tsx b/packages/query-core/src/tests/queryObserver.test.tsx index 550e5cbf91..286175d9af 100644 --- a/packages/query-core/src/tests/queryObserver.test.tsx +++ b/packages/query-core/src/tests/queryObserver.test.tsx @@ -461,13 +461,12 @@ describe('queryObserver', () => { }) observer.setOptions({ queryKey: key, enabled: false, staleTime: 10 }) await queryClient.fetchQuery({ queryKey: key, queryFn }) - await sleep(100) + await sleep(20) unsubscribe() expect(queryFn).toHaveBeenCalledTimes(1) - expect(results.length).toBe(3) - expect(results[0]).toMatchObject({ isStale: true }) - expect(results[1]).toMatchObject({ isStale: false }) - expect(results[2]).toMatchObject({ isStale: true }) + expect(results.length).toBe(2) + expect(results[0]).toMatchObject({ isStale: false, data: undefined }) + expect(results[1]).toMatchObject({ isStale: false, data: 'data' }) }) test('should be able to handle multiple subscribers', async () => { @@ -881,11 +880,12 @@ describe('queryObserver', () => { const observer = new QueryObserver(queryClient, { queryKey: key, + enabled: false, }) const spy = vi.fn() const unsubscribe = queryClient.getQueryCache().subscribe(spy) - observer.setOptions({ queryKey: key, enabled: false }) + observer.setOptions({ queryKey: key, enabled: false, refetchInterval: 10 }) expect(spy).toHaveBeenCalledTimes(1) expect(spy).toHaveBeenCalledWith( @@ -895,6 +895,18 @@ describe('queryObserver', () => { unsubscribe() }) + test('disabled observers should not be stale', async () => { + const key = queryKey() + + const observer = new QueryObserver(queryClient, { + queryKey: key, + enabled: false, + }) + + const result = observer.getCurrentResult() + expect(result.isStale).toBe(false) + }) + test('should be inferred as a correct result type', async () => { const key = queryKey() const data = { value: 'data' } diff --git a/packages/react-query/src/__tests__/useQuery.test.tsx b/packages/react-query/src/__tests__/useQuery.test.tsx index eec7607c9e..9ce1530820 100644 --- a/packages/react-query/src/__tests__/useQuery.test.tsx +++ b/packages/react-query/src/__tests__/useQuery.test.tsx @@ -1223,7 +1223,7 @@ describe('useQuery', () => { data: undefined, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) }) @@ -1248,7 +1248,7 @@ describe('useQuery', () => { React.useEffect(() => { setActTimeout(() => { queryClient.invalidateQueries({ queryKey: key }) - }, 20) + }, 10) }, []) return null @@ -1256,14 +1256,14 @@ describe('useQuery', () => { renderWithClient(queryClient, ) - await sleep(100) + await sleep(50) expect(states.length).toBe(1) expect(states[0]).toMatchObject({ data: undefined, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) }) @@ -3783,9 +3783,9 @@ describe('useQuery', () => {
FetchStatus: {query.fetchStatus}

Data: {query.data || 'no data'}

- {query.isStale ? ( + {shouldFetch ? null : ( - ) : null} + )}
) } @@ -3956,7 +3956,8 @@ describe('useQuery', () => { expect(results.length).toBe(3) expect(results[0]).toMatchObject({ data: 'initial', isStale: true }) expect(results[1]).toMatchObject({ data: 'fetched data', isStale: true }) - expect(results[2]).toMatchObject({ data: 'fetched data', isStale: true }) + // disabled observers are not stale + expect(results[2]).toMatchObject({ data: 'fetched data', isStale: false }) }) it('it should support enabled:false in query object syntax', async () => { @@ -4891,14 +4892,14 @@ describe('useQuery', () => { isPending: true, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) expect(states[1]).toMatchObject({ data: undefined, isPending: true, isFetching: true, isSuccess: false, - isStale: true, + isStale: false, }) expect(states[2]).toMatchObject({ data: 1, @@ -4912,7 +4913,7 @@ describe('useQuery', () => { isPending: true, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) }) From 3867eac8be12db489303d75129d3d4754f04d689 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sat, 9 Mar 2024 13:32:20 +0100 Subject: [PATCH 3/5] fix: solid-query tests with the new assumption that disabled observers are not stale --- .../src/__tests__/createQuery.test.tsx | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx index 7e21e5c0e3..1a6b0fc84c 100644 --- a/packages/solid-query/src/__tests__/createQuery.test.tsx +++ b/packages/solid-query/src/__tests__/createQuery.test.tsx @@ -1255,7 +1255,7 @@ describe('createQuery', () => { data: undefined, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) }) @@ -1301,7 +1301,7 @@ describe('createQuery', () => { data: undefined, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) }) @@ -3376,9 +3376,9 @@ describe('createQuery', () => {
FetchStatus: {query.fetchStatus}

Data: {query.data || 'no data'}

- {query.isStale ? ( + {shouldFetch() ? null : ( - ) : null} + )}
) } @@ -3502,10 +3502,11 @@ describe('createQuery', () => { )) await sleep(50) - expect(results.length).toBe(2) + expect(results.length).toBe(3) expect(results[0]).toMatchObject({ data: 'initial', isStale: true }) expect(results[1]).toMatchObject({ data: 'fetched data', isStale: true }) - // Wont render 3rd time, because data is still the same + // disabled observers are not stale + expect(results[2]).toMatchObject({ data: 'fetched data', isStale: false }) }) it('it should support enabled:false in query object syntax', async () => { @@ -4550,13 +4551,13 @@ describe('createQuery', () => { isPending: true, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) expect(states[1]).toMatchObject({ isPending: true, isFetching: true, isSuccess: false, - isStale: true, + isStale: false, }) expect(states[2]).toMatchObject({ data: 1, @@ -4569,7 +4570,7 @@ describe('createQuery', () => { isPending: true, isFetching: false, isSuccess: false, - isStale: true, + isStale: false, }) }) From d08148cf676af8799ba1705f2543ac3a1e01e2d3 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Sat, 9 Mar 2024 15:26:51 +0100 Subject: [PATCH 4/5] fix: re-add check accidentally removed to test a failing test --- packages/query-core/src/queryObserver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts index 4035c82064..6fd2694a10 100644 --- a/packages/query-core/src/queryObserver.ts +++ b/packages/query-core/src/queryObserver.ts @@ -741,7 +741,7 @@ function isStale( query: Query, options: QueryObserverOptions, ): boolean { - return query.isStaleByTime(options.staleTime) + return options.enabled !== false && query.isStaleByTime(options.staleTime) } // this function would decide if we will update the observer's 'current' From 014aedde064ec3c14cdf41d6151c5484d05b3e4f Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Mon, 11 Mar 2024 13:33:01 +0100 Subject: [PATCH 5/5] fix: prefer the stale value of observers if one exists that check looks for data not undefined anyways, but it also returns "fresh" for disabled observers now --- packages/query-core/src/query.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index fcca677bdd..dc1f703fdc 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -248,11 +248,17 @@ export class Query< } isStale(): boolean { - return ( - this.state.isInvalidated || - this.state.data === undefined || - this.#observers.some((observer) => observer.getCurrentResult().isStale) - ) + 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 {