fix(core): do not count disabled observers as stale (#7059)

* 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 * fix: react-query tests with the new assumption that disabled observers are not stale * fix: solid-query tests with the new assumption that disabled observers are not stale * fix: re-add check accidentally removed to test a failing test * 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
TanStack · Mar 12, 2024 · c55f609 · c55f609
1 parent a437f6d
commit c55f609
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 35 deletions.
diff --git 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 {

diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts
@@ -156,7 +156,10 @@ export class QueryObserver<
     this.#updateQuery()
     this.#currentQuery.setOptions(this.options)
 
-    if (!shallowEqualObjects(this.options, prevOptions)) {
+    if (
+      prevOptions._defaulted &&
+      !shallowEqualObjects(this.options, prevOptions)
+    ) {
       this.#client.getQueryCache().notify({
         type: 'observerOptionsUpdated',
         query: this.#currentQuery,
@@ -729,7 +732,6 @@ function shouldFetchOptionally(
   prevOptions: QueryObserverOptions<any, any, any, any, any>,
 ): boolean {
   return (
-    options.enabled !== false &&
     (query !== prevQuery || prevOptions.enabled === false) &&
     (!options.suspense || query.state.status !== 'error') &&
     isStale(query, options)
@@ -740,7 +742,7 @@ function isStale(
   query: Query<any, any, any, any>,
   options: QueryObserverOptions<any, any, any, any, any>,
 ): 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'

diff --git 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

diff --git a/packages/query-core/src/tests/queryObserver.test.tsx b/packages/query-core/src/tests/queryObserver.test.tsx
@@ -465,13 +465,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 () => {
@@ -885,11 +884,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(
@@ -898,4 +898,16 @@ 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)
+  })
 })
diff --git 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,22 +1248,22 @@ describe('useQuery', () => {
       React.useEffect(() => {
         setActTimeout(() => {
           queryClient.invalidateQueries({ queryKey: key })
-        }, 20)
+        }, 10)
       }, [])
 
       return null
     }
 
     renderWithClient(queryClient, <Page />)
 
-    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,
     })
   })
 
@@ -3778,9 +3778,9 @@ describe('useQuery', () => {
         <div>
           <div>FetchStatus: {query.fetchStatus}</div>
           <h2>Data: {query.data || 'no data'}</h2>
-          {query.isStale ? (
+          {shouldFetch ? null : (
             <button onClick={() => setShouldFetch(true)}>fetch</button>
-          ) : null}
+          )}
         </div>
       )
     }
@@ -3951,7 +3951,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 () => {
@@ -4886,14 +4887,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,
@@ -4907,7 +4908,7 @@ describe('useQuery', () => {
       isPending: true,
       isFetching: false,
       isSuccess: false,
-      isStale: true,
+      isStale: false,
     })
   })
 

diff --git a/packages/solid-query/src/__tests__/createQuery.test.tsx b/packages/solid-query/src/__tests__/createQuery.test.tsx
@@ -1259,7 +1259,7 @@ describe('createQuery', () => {
       data: undefined,
       isFetching: false,
       isSuccess: false,
-      isStale: true,
+      isStale: false,
     })
   })
 
@@ -1305,7 +1305,7 @@ describe('createQuery', () => {
       data: undefined,
       isFetching: false,
       isSuccess: false,
-      isStale: true,
+      isStale: false,
     })
   })
 
@@ -3375,9 +3375,9 @@ describe('createQuery', () => {
         <div>
           <div>FetchStatus: {query.fetchStatus}</div>
           <h2>Data: {query.data || 'no data'}</h2>
-          {query.isStale ? (
+          {shouldFetch() ? null : (
             <button onClick={() => setShouldFetch(true)}>fetch</button>
-          ) : null}
+          )}
         </div>
       )
     }
@@ -3501,10 +3501,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 () => {
@@ -4549,13 +4550,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,
@@ -4568,7 +4569,7 @@ describe('createQuery', () => {
       isPending: true,
       isFetching: false,
       isSuccess: false,
-      isStale: true,
+      isStale: false,
     })
   })