Skip to content

Commit 1b54538

Browse files
TkDodoTkDodo
authored
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
1 parent b23bb39 commit 1b54538

File tree

12 files changed

+93
-51
lines changed

12 files changed

+93
-51
lines changed

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import type { CreateBaseQueryOptions } from './types'
2020

2121
/**
2222
* Base implementation for `injectQuery` and `injectInfiniteQuery`.
23+
* @param optionsFn
24+
* @param Observer
2325
*/
2426
export function createBaseQuery<
2527
TQueryFnData,
@@ -82,11 +84,7 @@ export function createBaseQuery<
8284
const defaultedOptions = defaultedOptionsSignal()
8385

8486
untracked(() => {
85-
observer.setOptions(defaultedOptions, {
86-
// Do not notify on updates because of changes in the options because
87-
// these changes should already be reflected in the optimistic result.
88-
listeners: false,
89-
})
87+
observer.setOptions(defaultedOptions)
9088
})
9189
onCleanup(() => {
9290
ngZone.run(() => resultFromSubscriberSignal.set(null))

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,6 @@ export function injectQueries<
239239
observer.setQueries(
240240
defaultedQueries(),
241241
options as QueriesObserverOptions<TCombinedResult>,
242-
{ listeners: false },
243242
)
244243
})
245244

‎packages/query-core/src/infiniteQueryObserver.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import type {
1717
QueryKey,
1818
} from './types'
1919
import type { QueryClient } from './queryClient'
20-
import type { NotifyOptions } from './queryObserver'
2120
import type { Query } from './query'
2221

2322
type InfiniteQueryObserverListener<TData, TError> = (
@@ -96,15 +95,11 @@ export class InfiniteQueryObserver<
9695
TQueryKey,
9796
TPageParam
9897
>,
99-
notifyOptions?: NotifyOptions,
10098
): void {
101-
super.setOptions(
102-
{
103-
...options,
104-
behavior: infiniteQueryBehavior(),
105-
},
106-
notifyOptions,
107-
)
99+
super.setOptions({
100+
...options,
101+
behavior: infiniteQueryBehavior(),
102+
})
108103
}
109104

110105
getOptimisticResult(

‎packages/query-core/src/queriesObserver.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import type {
88
QueryObserverResult,
99
} from './types'
1010
import type { QueryClient } from './queryClient'
11-
import type { NotifyOptions } from './queryObserver'
1211

1312
function difference<T>(array1: Array<T>, array2: Array<T>): Array<T> {
1413
return array1.filter((x) => !array2.includes(x))
@@ -87,7 +86,6 @@ export class QueriesObserver<
8786
setQueries(
8887
queries: Array<QueryObserverOptions>,
8988
options?: QueriesObserverOptions<TCombinedResult>,
90-
notifyOptions?: NotifyOptions,
9189
): void {
9290
this.#queries = queries
9391
this.#options = options
@@ -111,7 +109,7 @@ export class QueriesObserver<
111109

112110
// set options for the new observers to notify of changes
113111
newObserverMatches.forEach((match) =>
114-
match.observer.setOptions(match.defaultedQueryOptions, notifyOptions),
112+
match.observer.setOptions(match.defaultedQueryOptions),
115113
)
116114

117115
const newObservers = newObserverMatches.map((match) => match.observer)

‎packages/query-core/src/queryObserver.ts‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,6 @@ type QueryObserverListener<TData, TError> = (
3232
result: QueryObserverResult<TData, TError>,
3333
) => void
3434

35-
export interface NotifyOptions {
36-
listeners?: boolean
37-
}
38-
3935
interface ObserverFetchOptions extends FetchOptions {
4036
throwOnError?: boolean
4137
}
@@ -151,7 +147,6 @@ export class QueryObserver<
151147
TQueryData,
152148
TQueryKey
153149
>,
154-
notifyOptions?: NotifyOptions,
155150
): void {
156151
const prevOptions = this.options
157152
const prevQuery = this.#currentQuery
@@ -200,7 +195,7 @@ export class QueryObserver<
200195
}
201196

202197
// Update result
203-
this.updateResult(notifyOptions)
198+
this.updateResult()
204199

205200
// Update stale interval if needed
206201
if (
@@ -648,7 +643,7 @@ export class QueryObserver<
648643
return nextResult
649644
}
650645

651-
updateResult(notifyOptions?: NotifyOptions): void {
646+
updateResult(): void {
652647
const prevResult = this.#currentResult as
653648
| QueryObserverResult<TData, TError>
654649
| undefined
@@ -669,9 +664,6 @@ export class QueryObserver<
669664

670665
this.#currentResult = nextResult
671666

672-
// Determine which callbacks to trigger
673-
const defaultNotifyOptions: NotifyOptions = {}
674-
675667
const shouldNotifyListeners = (): boolean => {
676668
if (!prevResult) {
677669
return true
@@ -706,11 +698,7 @@ export class QueryObserver<
706698
})
707699
}
708700

709-
if (notifyOptions?.listeners !== false && shouldNotifyListeners()) {
710-
defaultNotifyOptions.listeners = true
711-
}
712-
713-
this.#notify({ ...defaultNotifyOptions, ...notifyOptions })
701+
this.#notify({ listeners: shouldNotifyListeners() })
714702
}
715703

716704
#updateQuery(): void {
@@ -740,7 +728,7 @@ export class QueryObserver<
740728
}
741729
}
742730

743-
#notify(notifyOptions: NotifyOptions): void {
731+
#notify(notifyOptions: { listeners: boolean }): void {
744732
notifyManager.batch(() => {
745733
// First, trigger the listeners
746734
if (notifyOptions.listeners) {

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2165,6 +2165,64 @@ describe('useQuery', () => {
21652165
expect(states[2]).toMatchObject({ isStale: true })
21662166
})
21672167

2168+
it('should re-render disabled observers when other observers trigger a query (#8741)', async () => {
2169+
const key = queryKey()
2170+
2171+
const useUserInfoQuery = ({
2172+
id,
2173+
enabled,
2174+
}: {
2175+
id: number | null
2176+
enabled: boolean
2177+
}) => {
2178+
return useQuery({
2179+
queryKey: [key, id],
2180+
queryFn: async () => {
2181+
await sleep(10)
2182+
return { id, name: 'John' }
2183+
},
2184+
enabled: !!id && enabled,
2185+
})
2186+
}
2187+
2188+
const Page = () => {
2189+
const [id, setId] = React.useState<number | null>(null)
2190+
2191+
const searchQuery = useUserInfoQuery({ id, enabled: false })
2192+
2193+
return (
2194+
<>
2195+
<div>User fetching status is {searchQuery.fetchStatus}</div>
2196+
<UserInfo id={id} />
2197+
<button onClick={() => setId(42)}>
2198+
Set ID and trigger user load
2199+
</button>
2200+
</>
2201+
)
2202+
}
2203+
2204+
function UserInfo({ id }: { id: number | null }) {
2205+
const searchQuery = useUserInfoQuery({ id, enabled: true })
2206+
2207+
return <div>UserInfo data is {JSON.stringify(searchQuery.data)} </div>
2208+
}
2209+
2210+
const rendered = renderWithClient(queryClient, <Page />)
2211+
2212+
rendered.getByText('User fetching status is idle')
2213+
2214+
fireEvent.click(rendered.getByRole('button', { name: /set id/i }))
2215+
2216+
await waitFor(() => {
2217+
rendered.getByText('User fetching status is fetching')
2218+
})
2219+
await waitFor(() => {
2220+
rendered.getByText('UserInfo data is {"id":42,"name":"John"}')
2221+
})
2222+
2223+
rendered.getByText('User fetching status is idle')
2224+
})
2225+
21682226
describe('notifyOnChangeProps', () => {
21692227
it('should not re-render when it should only re-render only data change and the selected data did not change', async () => {
21702228
const key = queryKey()

‎packages/react-query/src/useBaseQuery.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,7 @@ export function useBaseQuery<
114114
)
115115

116116
React.useEffect(() => {
117-
// Do not notify on updates because of changes in the options because
118-
// these changes should already be reflected in the optimistic result.
119-
observer.setOptions(defaultedOptions, { listeners: false })
117+
observer.setOptions(defaultedOptions)
120118
}, [defaultedOptions, observer])
121119

122120
// Handle suspense

‎packages/react-query/src/useQueries.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -279,14 +279,9 @@ export function useQueries<
279279
)
280280

281281
React.useEffect(() => {
282-
// Do not notify on updates because of changes in the options because
283-
// these changes should already be reflected in the optimistic result.
284282
observer.setQueries(
285283
defaultedQueries,
286284
options as QueriesObserverOptions<TCombinedResult>,
287-
{
288-
listeners: false,
289-
},
290285
)
291286
}, [defaultedQueries, options, observer])
292287

‎packages/solid-query/src/createQueries.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,6 @@ export function createQueries<
309309
combine: queriesOptions().combine,
310310
} as QueriesObserverOptions<TCombinedResult>)
311311
: undefined,
312-
{ listeners: false },
313312
)
314313
})
315314

@@ -321,7 +320,6 @@ export function createQueries<
321320
combine: queriesOptions().combine,
322321
} as QueriesObserverOptions<TCombinedResult>)
323322
: undefined,
324-
{ listeners: false },
325323
)
326324
})
327325

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

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ describe('PersistQueryClientProvider', () => {
8383
await waitFor(() => rendered.getByText('fetched'))
8484

8585
const states = get(statesStore)
86-
expect(states).toHaveLength(4)
86+
expect(states).toHaveLength(5)
8787

8888
expect(states[0]).toMatchObject({
8989
status: 'pending',
@@ -104,6 +104,12 @@ describe('PersistQueryClientProvider', () => {
104104
})
105105

106106
expect(states[3]).toMatchObject({
107+
status: 'success',
108+
fetchStatus: 'fetching',
109+
data: 'hydrated',
110+
})
111+
112+
expect(states[4]).toMatchObject({
107113
status: 'success',
108114
fetchStatus: 'idle',
109115
data: 'fetched',
@@ -139,7 +145,7 @@ describe('PersistQueryClientProvider', () => {
139145

140146
const states = get(statesStore)
141147

142-
expect(states).toHaveLength(4)
148+
expect(states).toHaveLength(5)
143149

144150
expect(states[0]).toMatchObject({
145151
status: 'pending',
@@ -160,6 +166,12 @@ describe('PersistQueryClientProvider', () => {
160166
})
161167

162168
expect(states[3]).toMatchObject({
169+
status: 'success',
170+
fetchStatus: 'fetching',
171+
data: 'hydrated',
172+
})
173+
174+
expect(states[4]).toMatchObject({
163175
status: 'success',
164176
fetchStatus: 'idle',
165177
data: 'fetched',
@@ -194,7 +206,7 @@ describe('PersistQueryClientProvider', () => {
194206
await waitFor(() => rendered.getByText('fetched'))
195207

196208
const states = get(statesStore)
197-
expect(states).toHaveLength(4)
209+
expect(states).toHaveLength(5)
198210

199211
expect(states[0]).toMatchObject({
200212
status: 'success',
@@ -215,6 +227,12 @@ describe('PersistQueryClientProvider', () => {
215227
})
216228

217229
expect(states[3]).toMatchObject({
230+
status: 'success',
231+
fetchStatus: 'fetching',
232+
data: 'hydrated',
233+
})
234+
235+
expect(states[4]).toMatchObject({
218236
status: 'success',
219237
fetchStatus: 'idle',
220238
data: 'fetched',

0 commit comments

Comments
 (0)