fix: notify on updates between mounting and subscribing (#1737)

Author: boschni

src/core/infiniteQueryObserver.ts

@@ -98,11 +98,9 @@ export class InfiniteQueryObserver<
     })
   }
 
-  protected getNewResult(
-    willFetch?: boolean
-  ): InfiniteQueryObserverResult<TData, TError> {
+  protected getNewResult(): InfiniteQueryObserverResult<TData, TError> {
     const { state } = this.getCurrentQuery()
-    const result = super.getNewResult(willFetch)
+    const result = super.getNewResult()
     return {
       ...result,
       fetchNextPage: this.fetchNextPage,

src/core/queryObserver.ts

@@ -87,6 +87,7 @@ export class QueryObserver<
         this.executeFetch()
       }
 
+      this.updateResult()
       this.updateTimers()
     }
   }
@@ -155,7 +156,6 @@ export class QueryObserver<
     options?: QueryObserverOptions<TQueryFnData, TError, TData, TQueryData>
   ): void {
     const prevOptions = this.options
-    const prevQuery = this.currentQuery
 
     this.options = this.client.defaultQueryObserverOptions(options)
 
@@ -171,39 +171,60 @@ export class QueryObserver<
       this.options.queryKey = prevOptions.queryKey
     }
 
-    this.updateQuery()
+    const didUpdateQuery = this.updateQuery()
 
-    // Take no further actions if there are no subscribers
-    if (!this.listeners.length) {
-      return
-    }
+    let optionalFetch
+    let updateStaleTimeout
+    let updateRefetchInterval
 
-    // If we subscribed to a new query, optionally fetch and update refetch
-    if (this.currentQuery !== prevQuery) {
-      this.optionalFetch()
-      this.updateTimers()
-      return
+    // If we subscribed to a new query, optionally fetch and update intervals
+    if (didUpdateQuery) {
+      optionalFetch = true
+      updateStaleTimeout = true
+      updateRefetchInterval = true
     }
 
     // Optionally fetch if the query became enabled
     if (this.options.enabled !== false && prevOptions.enabled === false) {
-      this.optionalFetch()
+      optionalFetch = true
     }
 
     // Update stale interval if needed
     if (
       this.options.enabled !== prevOptions.enabled ||
       this.options.staleTime !== prevOptions.staleTime
     ) {
-      this.updateStaleTimeout()
+      updateStaleTimeout = true
     }
 
     // Update refetch interval if needed
     if (
       this.options.enabled !== prevOptions.enabled ||
       this.options.refetchInterval !== prevOptions.refetchInterval
     ) {
-      this.updateRefetchInterval()
+      updateRefetchInterval = true
+    }
+
+    // Fetch only if there are subscribers
+    if (this.hasListeners()) {
+      if (optionalFetch) {
+        this.optionalFetch()
+      }
+    }
+
+    // Update result when subscribing to a new query
+    if (didUpdateQuery) {
+      this.updateResult()
+    }
+
+    // Update intervals only if there are subscribers
+    if (this.hasListeners()) {
+      if (updateStaleTimeout) {
+        this.updateStaleTimeout()
+      }
+      if (updateRefetchInterval) {
+        this.updateRefetchInterval()
+      }
     }
   }
 
@@ -302,12 +323,7 @@ export class QueryObserver<
 
     this.staleTimeoutId = setTimeout(() => {
       if (!this.currentResult.isStale) {
-        const prevResult = this.currentResult
         this.updateResult()
-        this.notify({
-          listeners: this.shouldNotifyListeners(prevResult, this.currentResult),
-          cache: true,
-        })
       }
     }, timeout)
   }
@@ -353,9 +369,7 @@ export class QueryObserver<
     this.refetchIntervalId = undefined
   }
 
-  protected getNewResult(
-    willFetch?: boolean
-  ): QueryObserverResult<TData, TError> {
+  protected getNewResult(): QueryObserverResult<TData, TError> {
     const { state } = this.currentQuery
     let { isFetching, status } = state
     let isPreviousData = false
@@ -364,7 +378,7 @@ export class QueryObserver<
     let dataUpdatedAt = state.dataUpdatedAt
 
     // Optimistically set status to loading if we will start fetching
-    if (willFetch) {
+    if (!this.hasListeners() && this.willFetchOnMount()) {
       isFetching = true
       if (!dataUpdatedAt) {
         status = 'loading'
@@ -442,7 +456,7 @@ export class QueryObserver<
   }
 
   private shouldNotifyListeners(
-    prevResult: QueryObserverResult,
+    prevResult: QueryObserverResult | undefined,
     result: QueryObserverResult
   ): boolean {
     const { notifyOnChangeProps, notifyOnChangePropsExclusions } = this.options
@@ -451,6 +465,10 @@ export class QueryObserver<
       return false
     }
 
+    if (!prevResult) {
+      return true
+    }
+
     if (!notifyOnChangeProps && !notifyOnChangePropsExclusions) {
       return true
     }
@@ -485,39 +503,60 @@ export class QueryObserver<
     return false
   }
 
-  private updateResult(willFetch?: boolean): void {
-    const result = this.getNewResult(willFetch)
+  private updateResult(action?: Action<TData, TError>): void {
+    const prevResult = this.currentResult as
+      | QueryObserverResult<TData, TError>
+      | undefined
+
+    const result = this.getNewResult()
 
     // Keep reference to the current state on which the current result is based on
     this.currentResultState = this.currentQuery.state
 
     // Only update if something has changed
-    if (!shallowEqualObjects(result, this.currentResult)) {
-      this.currentResult = result
+    if (shallowEqualObjects(result, prevResult)) {
+      return
+    }
 
-      if (this.options.notifyOnChangeProps === 'tracked') {
-        const addTrackedProps = (prop: keyof QueryObserverResult) => {
-          if (!this.trackedProps.includes(prop)) {
-            this.trackedProps.push(prop)
-          }
+    this.currentResult = result
+
+    if (this.options.notifyOnChangeProps === 'tracked') {
+      const addTrackedProps = (prop: keyof QueryObserverResult) => {
+        if (!this.trackedProps.includes(prop)) {
+          this.trackedProps.push(prop)
         }
-        this.trackedCurrentResult = {} as QueryObserverResult<TData, TError>
-
-        Object.keys(result).forEach(key => {
-          Object.defineProperty(this.trackedCurrentResult, key, {
-            configurable: false,
-            enumerable: true,
-            get() {
-              addTrackedProps(key as keyof QueryObserverResult)
-              return result[key as keyof QueryObserverResult]
-            },
-          })
-        })
       }
+      this.trackedCurrentResult = {} as QueryObserverResult<TData, TError>
+
+      Object.keys(result).forEach(key => {
+        Object.defineProperty(this.trackedCurrentResult, key, {
+          configurable: false,
+          enumerable: true,
+          get() {
+            addTrackedProps(key as keyof QueryObserverResult)
+            return result[key as keyof QueryObserverResult]
+          },
+        })
+      })
+    }
+
+    // Determine which callbacks to trigger
+    const notifyOptions: NotifyOptions = { cache: true }
+
+    if (action?.type === 'success') {
+      notifyOptions.onSuccess = true
+    } else if (action?.type === 'error') {
+      notifyOptions.onError = true
     }
+
+    if (this.shouldNotifyListeners(prevResult, result)) {
+      notifyOptions.listeners = true
+    }
+
+    this.notify(notifyOptions)
   }
 
-  private updateQuery(): void {
+  private updateQuery(): boolean {
     const prevQuery = this.currentQuery
 
     const query = this.client
@@ -528,62 +567,27 @@ export class QueryObserver<
       )
 
     if (query === prevQuery) {
-      return
+      return false
     }
 
     this.previousQueryResult = this.currentResult
     this.currentQuery = query
     this.initialDataUpdateCount = query.state.dataUpdateCount
     this.initialErrorUpdateCount = query.state.errorUpdateCount
 
-    const willFetch = prevQuery
-      ? this.willFetchOptionally()
-      : this.willFetchOnMount()
-
-    this.updateResult(willFetch)
-
-    if (!this.hasListeners()) {
-      return
+    if (this.hasListeners()) {
+      prevQuery?.removeObserver(this)
+      this.currentQuery.addObserver(this)
     }
 
-    prevQuery?.removeObserver(this)
-    this.currentQuery.addObserver(this)
-
-    if (
-      this.shouldNotifyListeners(this.previousQueryResult, this.currentResult)
-    ) {
-      this.notify({ listeners: true })
-    }
+    return true
   }
 
   onQueryUpdate(action: Action<TData, TError>): void {
-    // Store current result and get new result
-    const prevResult = this.currentResult
-    this.updateResult()
-    const currentResult = this.currentResult
-
-    // Update timers
-    this.updateTimers()
-
-    // Do not notify if the nothing has changed
-    if (prevResult === currentResult) {
-      return
-    }
-
-    // Determine which callbacks to trigger
-    const notifyOptions: NotifyOptions = {}
-
-    if (action.type === 'success') {
-      notifyOptions.onSuccess = true
-    } else if (action.type === 'error') {
-      notifyOptions.onError = true
-    }
-
-    if (this.shouldNotifyListeners(prevResult, currentResult)) {
-      notifyOptions.listeners = true
+    this.updateResult(action)
+    if (this.hasListeners()) {
+      this.updateTimers()
     }
-
-    this.notify(notifyOptions)
   }
 
   private notify(notifyOptions: NotifyOptions): void {

src/core/tests/queryClient.test.tsx

@@ -77,6 +77,7 @@ describe('queryClient', () => {
       const observer = new QueryObserver(queryClient, {
         queryKey: [key],
         retry: false,
+        enabled: false,
       })
       const { status } = await observer.refetch()
       expect(status).toBe('error')

src/core/tests/queryObserver.test.tsx

@@ -49,6 +49,24 @@ describe('queryObserver', () => {
     expect(results[2]).toMatchObject({ data: 2, status: 'success' })
   })
 
+  test('should notify when the query has updated before subscribing', async () => {
+    const key = queryKey()
+    const results: QueryObserverResult[] = []
+    const observer = new QueryObserver(queryClient, {
+      queryKey: key,
+      queryFn: () => 1,
+      staleTime: Infinity,
+    })
+    queryClient.setQueryData(key, 2)
+    const unsubscribe = observer.subscribe(result => {
+      results.push(result)
+    })
+    await sleep(1)
+    unsubscribe()
+    expect(results.length).toBe(1)
+    expect(results[0]).toMatchObject({ data: 2, status: 'success' })
+  })
+
   test('should be able to fetch with a selector', async () => {
     const key = queryKey()
     const observer = new QueryObserver(queryClient, {