fix(core): make sure that calling setOptions will always notifyListeners (#8771)

* fix(core): make sure that calling setOptions will always notifyListeners

we have an optimized `shouldNotifyListeners` logic internally in the QueryObserver that will make sure we only notify listeners that are interested in a change because they are using a specific key (tracked query properties)

the optimization to force a skip of the notification when updating options because the optimistic result should already contain all the necessary output falls apart in the situation described in the test and in the linked issue #8741.

while the issue itself is a niche edge-case, the fix is to actually remove a bunch of code, so I'm all for it

* chore: fix other adapters

* chore: fix svelte, too

* chore: fix svelte tests

Author: TkDodo

packages/angular-query-experimental/src/create-base-query.ts

@@ -20,6 +20,8 @@ import type { CreateBaseQueryOptions } from './types'
 
 /**
  * Base implementation for `injectQuery` and `injectInfiniteQuery`.
+ * @param optionsFn
+ * @param Observer
  */
 export function createBaseQuery<
   TQueryFnData,
@@ -82,11 +84,7 @@ export function createBaseQuery<
       const defaultedOptions = defaultedOptionsSignal()
 
       untracked(() => {
-        observer.setOptions(defaultedOptions, {
-          // Do not notify on updates because of changes in the options because
-          // these changes should already be reflected in the optimistic result.
-          listeners: false,
-        })
+        observer.setOptions(defaultedOptions)
       })
       onCleanup(() => {
         ngZone.run(() => resultFromSubscriberSignal.set(null))

packages/angular-query-experimental/src/inject-queries.ts

@@ -239,7 +239,6 @@ export function injectQueries<
       observer.setQueries(
         defaultedQueries(),
         options as QueriesObserverOptions<TCombinedResult>,
-        { listeners: false },
       )
     })
 

packages/query-core/src/infiniteQueryObserver.ts

@@ -17,7 +17,6 @@ import type {
   QueryKey,
 } from './types'
 import type { QueryClient } from './queryClient'
-import type { NotifyOptions } from './queryObserver'
 import type { Query } from './query'
 
 type InfiniteQueryObserverListener<TData, TError> = (
@@ -96,15 +95,11 @@ export class InfiniteQueryObserver<
       TQueryKey,
       TPageParam
     >,
-    notifyOptions?: NotifyOptions,
   ): void {
-    super.setOptions(
-      {
-        ...options,
-        behavior: infiniteQueryBehavior(),
-      },
-      notifyOptions,
-    )
+    super.setOptions({
+      ...options,
+      behavior: infiniteQueryBehavior(),
+    })
   }
 
   getOptimisticResult(

packages/query-core/src/queriesObserver.ts

@@ -8,7 +8,6 @@ import type {
   QueryObserverResult,
 } from './types'
 import type { QueryClient } from './queryClient'
-import type { NotifyOptions } from './queryObserver'
 
 function difference<T>(array1: Array<T>, array2: Array<T>): Array<T> {
   return array1.filter((x) => !array2.includes(x))
@@ -87,7 +86,6 @@ export class QueriesObserver<
   setQueries(
     queries: Array<QueryObserverOptions>,
     options?: QueriesObserverOptions<TCombinedResult>,
-    notifyOptions?: NotifyOptions,
   ): void {
     this.#queries = queries
     this.#options = options
@@ -111,7 +109,7 @@ export class QueriesObserver<
 
       // set options for the new observers to notify of changes
       newObserverMatches.forEach((match) =>
-        match.observer.setOptions(match.defaultedQueryOptions, notifyOptions),
+        match.observer.setOptions(match.defaultedQueryOptions),
       )
 
       const newObservers = newObserverMatches.map((match) => match.observer)

packages/query-core/src/queryObserver.ts

@@ -32,10 +32,6 @@ type QueryObserverListener<TData, TError> = (
   result: QueryObserverResult<TData, TError>,
 ) => void
 
-export interface NotifyOptions {
-  listeners?: boolean
-}
-
 interface ObserverFetchOptions extends FetchOptions {
   throwOnError?: boolean
 }
@@ -151,7 +147,6 @@ export class QueryObserver<
       TQueryData,
       TQueryKey
     >,
-    notifyOptions?: NotifyOptions,
   ): void {
     const prevOptions = this.options
     const prevQuery = this.#currentQuery
@@ -200,7 +195,7 @@ export class QueryObserver<
     }
 
     // Update result
-    this.updateResult(notifyOptions)
+    this.updateResult()
 
     // Update stale interval if needed
     if (
@@ -648,7 +643,7 @@ export class QueryObserver<
     return nextResult
   }
 
-  updateResult(notifyOptions?: NotifyOptions): void {
+  updateResult(): void {
     const prevResult = this.#currentResult as
       | QueryObserverResult<TData, TError>
       | undefined
@@ -669,9 +664,6 @@ export class QueryObserver<
 
     this.#currentResult = nextResult
 
-    // Determine which callbacks to trigger
-    const defaultNotifyOptions: NotifyOptions = {}
-
     const shouldNotifyListeners = (): boolean => {
       if (!prevResult) {
         return true
@@ -706,11 +698,7 @@ export class QueryObserver<
       })
     }
 
-    if (notifyOptions?.listeners !== false && shouldNotifyListeners()) {
-      defaultNotifyOptions.listeners = true
-    }
-
-    this.#notify({ ...defaultNotifyOptions, ...notifyOptions })
+    this.#notify({ listeners: shouldNotifyListeners() })
   }
 
   #updateQuery(): void {
@@ -740,7 +728,7 @@ export class QueryObserver<
     }
   }
 
-  #notify(notifyOptions: NotifyOptions): void {
+  #notify(notifyOptions: { listeners: boolean }): void {
     notifyManager.batch(() => {
       // First, trigger the listeners
       if (notifyOptions.listeners) {

packages/react-query/src/__tests__/useQuery.test.tsx

@@ -2165,6 +2165,64 @@ describe('useQuery', () => {
     expect(states[2]).toMatchObject({ isStale: true })
   })
 
+  it('should re-render disabled observers when other observers trigger a query (#8741)', async () => {
+    const key = queryKey()
+
+    const useUserInfoQuery = ({
+      id,
+      enabled,
+    }: {
+      id: number | null
+      enabled: boolean
+    }) => {
+      return useQuery({
+        queryKey: [key, id],
+        queryFn: async () => {
+          await sleep(10)
+          return { id, name: 'John' }
+        },
+        enabled: !!id && enabled,
+      })
+    }
+
+    const Page = () => {
+      const [id, setId] = React.useState<number | null>(null)
+
+      const searchQuery = useUserInfoQuery({ id, enabled: false })
+
+      return (
+        <>
+          <div>User fetching status is {searchQuery.fetchStatus}</div>
+          <UserInfo id={id} />
+          <button onClick={() => setId(42)}>
+            Set ID and trigger user load
+          </button>
+        </>
+      )
+    }
+
+    function UserInfo({ id }: { id: number | null }) {
+      const searchQuery = useUserInfoQuery({ id, enabled: true })
+
+      return <div>UserInfo data is {JSON.stringify(searchQuery.data)} </div>
+    }
+
+    const rendered = renderWithClient(queryClient, <Page />)
+
+    rendered.getByText('User fetching status is idle')
+
+    fireEvent.click(rendered.getByRole('button', { name: /set id/i }))
+
+    await waitFor(() => {
+      rendered.getByText('User fetching status is fetching')
+    })
+    await waitFor(() => {
+      rendered.getByText('UserInfo data is {"id":42,"name":"John"}')
+    })
+
+    rendered.getByText('User fetching status is idle')
+  })
+
   describe('notifyOnChangeProps', () => {
     it('should not re-render when it should only re-render only data change and the selected data did not change', async () => {
       const key = queryKey()

packages/react-query/src/useBaseQuery.ts

@@ -114,9 +114,7 @@ export function useBaseQuery<
   )
 
   React.useEffect(() => {
-    // Do not notify on updates because of changes in the options because
-    // these changes should already be reflected in the optimistic result.
-    observer.setOptions(defaultedOptions, { listeners: false })
+    observer.setOptions(defaultedOptions)
   }, [defaultedOptions, observer])
 
   // Handle suspense

packages/react-query/src/useQueries.ts

@@ -279,14 +279,9 @@ export function useQueries<
   )
 
   React.useEffect(() => {
-    // Do not notify on updates because of changes in the options because
-    // these changes should already be reflected in the optimistic result.
     observer.setQueries(
       defaultedQueries,
       options as QueriesObserverOptions<TCombinedResult>,
-      {
-        listeners: false,
-      },
     )
   }, [defaultedQueries, options, observer])
 

packages/solid-query/src/createQueries.ts

@@ -309,7 +309,6 @@ export function createQueries<
             combine: queriesOptions().combine,
           } as QueriesObserverOptions<TCombinedResult>)
         : undefined,
-      { listeners: false },
     )
   })
 
@@ -321,7 +320,6 @@ export function createQueries<
             combine: queriesOptions().combine,
           } as QueriesObserverOptions<TCombinedResult>)
         : undefined,
-      { listeners: false },
     )
   })
 

packages/svelte-query-persist-client/tests/PersistQueryClientProvider.test.ts

@@ -83,7 +83,7 @@ describe('PersistQueryClientProvider', () => {
     await waitFor(() => rendered.getByText('fetched'))
 
     const states = get(statesStore)
-    expect(states).toHaveLength(4)
+    expect(states).toHaveLength(5)
 
     expect(states[0]).toMatchObject({
       status: 'pending',
@@ -104,6 +104,12 @@ describe('PersistQueryClientProvider', () => {
     })
 
     expect(states[3]).toMatchObject({
+      status: 'success',
+      fetchStatus: 'fetching',
+      data: 'hydrated',
+    })
+
+    expect(states[4]).toMatchObject({
       status: 'success',
       fetchStatus: 'idle',
       data: 'fetched',
@@ -139,7 +145,7 @@ describe('PersistQueryClientProvider', () => {
 
     const states = get(statesStore)
 
-    expect(states).toHaveLength(4)
+    expect(states).toHaveLength(5)
 
     expect(states[0]).toMatchObject({
       status: 'pending',
@@ -160,6 +166,12 @@ describe('PersistQueryClientProvider', () => {
     })
 
     expect(states[3]).toMatchObject({
+      status: 'success',
+      fetchStatus: 'fetching',
+      data: 'hydrated',
+    })
+
+    expect(states[4]).toMatchObject({
       status: 'success',
       fetchStatus: 'idle',
       data: 'fetched',
@@ -194,7 +206,7 @@ describe('PersistQueryClientProvider', () => {
     await waitFor(() => rendered.getByText('fetched'))
 
     const states = get(statesStore)
-    expect(states).toHaveLength(4)
+    expect(states).toHaveLength(5)
 
     expect(states[0]).toMatchObject({
       status: 'success',
@@ -215,6 +227,12 @@ describe('PersistQueryClientProvider', () => {
     })
 
     expect(states[3]).toMatchObject({
+      status: 'success',
+      fetchStatus: 'fetching',
+      data: 'hydrated',
+    })
+
+    expect(states[4]).toMatchObject({
       status: 'success',
       fetchStatus: 'idle',
       data: 'fetched',

packages/svelte-query/src/createBaseQuery.ts

@@ -56,9 +56,7 @@ export function createBaseQuery<
   >(client, get(defaultedOptionsStore))
 
   defaultedOptionsStore.subscribe(($defaultedOptions) => {
-    // Do not notify on updates because of changes in the options because
-    // these changes should already be reflected in the optimistic result.
-    observer.setOptions($defaultedOptions, { listeners: false })
+    observer.setOptions($defaultedOptions)
   })
 
   const result = derived<

packages/svelte-query/src/createQueries.ts

@@ -235,7 +235,6 @@ export function createQueries<
     observer.setQueries(
       $defaultedQueries,
       options as QueriesObserverOptions<TCombinedResult>,
-      { listeners: false },
     )
   })