Skip to content

Commit af0e0ef

Browse files
authored
fix(query-core): stop cancelled streamed queries from updating the cache (#12481)
* test(query-core): cover streamed query cancellation without signal consumption * fix(query-core): observe streamed fetch cancellation without consuming the signal * refactor(query-core): store fetch signals on contexts * refactor(query-core): remove copied-context signal fallback * fix(query-core): preserve fetch signals across context copies * fix(query-core): hide callback signal metadata from object rest
1 parent d0ff9ae commit af0e0ef

9 files changed

Lines changed: 260 additions & 40 deletions

File tree

‎.changeset/tidy-streams-stop.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/query-core': patch
3+
---
4+
5+
Prevent cancelled streamed queries from writing to the cache or starting a delayed stream, even when the stream does not consume the abort signal.

‎packages/angular-query-experimental/src/__tests__/inject-query.test.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,15 @@ describe('injectQuery', () => {
209209
TestBed.tick()
210210
expect(queryFn).toHaveBeenCalledTimes(2)
211211
// should call queryFn with context containing the new queryKey
212-
expect(queryFn).toHaveBeenNthCalledWith(2, {
213-
client: queryClient,
214-
meta: undefined,
215-
queryKey: key2,
216-
signal: expect.anything(),
217-
})
212+
expect(queryFn).toHaveBeenNthCalledWith(
213+
2,
214+
expect.objectContaining({
215+
client: queryClient,
216+
meta: undefined,
217+
queryKey: key2,
218+
signal: expect.anything(),
219+
}),
220+
)
218221
})
219222

220223
it('should only run query once enabled signal is set to true', async () => {

‎packages/query-core/src/__tests__/infiniteQueryBehavior.test.tsx‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ describe('InfiniteQueryBehavior', () => {
8888
data: { pages: [1], pageParams: [1] },
8989
})
9090

91-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
91+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
9292
queryKey: key,
9393
client: queryClient,
9494
pageParam: 1,
@@ -101,7 +101,7 @@ describe('InfiniteQueryBehavior', () => {
101101

102102
// Fetch the second page
103103
await observer.fetchNextPage()
104-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
104+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
105105
queryKey: key,
106106
client: queryClient,
107107
pageParam: 2,
@@ -119,7 +119,7 @@ describe('InfiniteQueryBehavior', () => {
119119

120120
// Fetch the page before the first page
121121
await observer.fetchPreviousPage()
122-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
122+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
123123
queryKey: key,
124124
client: queryClient,
125125
pageParam: 0,
@@ -138,7 +138,7 @@ describe('InfiniteQueryBehavior', () => {
138138

139139
// Fetch the page before
140140
await observer.fetchPreviousPage()
141-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
141+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
142142
queryKey: key,
143143
client: queryClient,
144144
pageParam: -1,
@@ -156,7 +156,7 @@ describe('InfiniteQueryBehavior', () => {
156156

157157
// Fetch the page after
158158
await observer.fetchNextPage()
159-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
159+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
160160
queryKey: key,
161161
client: queryClient,
162162
pageParam: 1,
@@ -178,7 +178,7 @@ describe('InfiniteQueryBehavior', () => {
178178
// Only 2 pages should refetch
179179
expect(queryFnSpy).toHaveBeenCalledTimes(2)
180180

181-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
181+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
182182
queryKey: key,
183183
client: queryClient,
184184
pageParam: 0,
@@ -187,7 +187,7 @@ describe('InfiniteQueryBehavior', () => {
187187
signal: abortSignal,
188188
})
189189

190-
expect(queryFnSpy).toHaveBeenNthCalledWith(2, {
190+
expect(queryFnSpy.mock.calls[1]?.[0]).toMatchObject({
191191
queryKey: key,
192192
client: queryClient,
193193
pageParam: 1,
@@ -241,7 +241,7 @@ describe('InfiniteQueryBehavior', () => {
241241

242242
expect(queryFnSpy).toHaveBeenCalledTimes(1)
243243

244-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
244+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
245245
queryKey: key,
246246
client: queryClient,
247247
pageParam: 1,
@@ -298,7 +298,7 @@ describe('InfiniteQueryBehavior', () => {
298298

299299
expect(queryFnSpy).toHaveBeenCalledTimes(1)
300300

301-
expect(queryFnSpy).toHaveBeenNthCalledWith(1, {
301+
expect(queryFnSpy.mock.calls[0]?.[0]).toMatchObject({
302302
queryKey: key,
303303
client: queryClient,
304304
pageParam: 2,

‎packages/query-core/src/__tests__/streamedQuery.test.tsx‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,170 @@ describe('streamedQuery', () => {
396396
})
397397
})
398398

399+
it.each(['append', 'replace'] as const)(
400+
'should stop %s writes after explicit cancellation when the stream ignores the signal',
401+
async (refetchMode) => {
402+
const key = createKey()
403+
queryClient.setQueryData(key, [99])
404+
const observer = new QueryObserver(queryClient, {
405+
queryKey: key,
406+
queryFn: streamedQuery({
407+
streamFn: () => createAsyncNumberGenerator(3),
408+
refetchMode,
409+
}),
410+
persister: (queryFn, context) => queryFn({ ...context }),
411+
})
412+
413+
const unsubscribe = observer.subscribe(vi.fn())
414+
await vi.advanceTimersByTimeAsync(50)
415+
await queryClient.cancelQueries({ queryKey: key })
416+
const cancelledData = queryClient.getQueryData(key)
417+
expect(cancelledData).toStrictEqual(
418+
refetchMode === 'append' ? [99, 0] : [99],
419+
)
420+
421+
await vi.advanceTimersByTimeAsync(150)
422+
expect(queryClient.getQueryData(key)).toStrictEqual(cancelledData)
423+
unsubscribe()
424+
},
425+
)
426+
427+
it('should preserve a non-streamed replacement when the old stream ignores the signal', async () => {
428+
const key = createKey()
429+
const queryFn = streamedQuery({
430+
streamFn: () => createAsyncNumberGenerator(3),
431+
})
432+
const observer = new QueryObserver(queryClient, {
433+
queryKey: key,
434+
queryFn: (context) => queryFn(context),
435+
})
436+
437+
const unsubscribe = observer.subscribe(vi.fn())
438+
await vi.advanceTimersByTimeAsync(50)
439+
observer.setOptions({ queryKey: key, queryFn: () => [100] })
440+
await observer.refetch()
441+
await vi.advanceTimersByTimeAsync(150)
442+
443+
expect(queryClient.getQueryData(key)).toStrictEqual([100])
444+
unsubscribe()
445+
})
446+
447+
it('should preserve the new stream when the old stream ignores the signal', async () => {
448+
const key = createKey()
449+
let offset = 0
450+
const observer = new QueryObserver(queryClient, {
451+
queryKey: key,
452+
queryFn: streamedQuery({
453+
streamFn: () => {
454+
const stream = createAsyncNumberGenerator(3, offset)
455+
offset += 100
456+
return stream
457+
},
458+
}),
459+
})
460+
461+
const unsubscribe = observer.subscribe(vi.fn())
462+
await vi.advanceTimersByTimeAsync(50)
463+
void observer.refetch()
464+
await vi.advanceTimersByTimeAsync(150)
465+
466+
expect(queryClient.getQueryData(key)).toStrictEqual([100, 101, 102])
467+
unsubscribe()
468+
})
469+
470+
it('should ignore a chunk queued just before cancellation', async () => {
471+
const key = createKey()
472+
queryClient.setQueryData(key, [99])
473+
let resolveChunk: (chunk: IteratorResult<number>) => void = () => {
474+
throw new Error('The iterator has not started')
475+
}
476+
const next = vi.fn<() => Promise<IteratorResult<number>>>(
477+
() =>
478+
new Promise<IteratorResult<number>>((resolve) => {
479+
resolveChunk = resolve
480+
}),
481+
)
482+
const observer = new QueryObserver(queryClient, {
483+
queryKey: key,
484+
queryFn: streamedQuery({
485+
refetchMode: 'append',
486+
streamFn: () => ({
487+
[Symbol.asyncIterator]: () => ({ next }),
488+
}),
489+
}),
490+
})
491+
492+
const unsubscribe = observer.subscribe(vi.fn())
493+
await vi.advanceTimersByTimeAsync(0)
494+
resolveChunk({ done: false, value: 1 })
495+
void queryClient.cancelQueries({ queryKey: key })
496+
await vi.advanceTimersByTimeAsync(0)
497+
498+
expect(queryClient.getQueryData(key)).toStrictEqual([99])
499+
expect(next).toHaveBeenCalledTimes(1)
500+
unsubscribe()
501+
})
502+
503+
it.each(['queryFn', 'persister'] as const)(
504+
'should not start a stream after cancellation during a delayed %s',
505+
async (delayIn) => {
506+
const key = createKey()
507+
queryClient.setQueryData(key, [99])
508+
const streamFn = vi.fn<() => AsyncIterable<number>>(() =>
509+
createAsyncNumberGenerator(1),
510+
)
511+
const queryFn = streamedQuery({ streamFn })
512+
const observer = new QueryObserver(queryClient, {
513+
queryKey: key,
514+
queryFn:
515+
delayIn === 'queryFn'
516+
? async (context) => {
517+
await sleep(50)
518+
return queryFn(context)
519+
}
520+
: queryFn,
521+
persister:
522+
delayIn === 'persister'
523+
? async (fn, context) => {
524+
await sleep(50)
525+
return fn({ ...context })
526+
}
527+
: undefined,
528+
})
529+
530+
const unsubscribe = observer.subscribe(vi.fn())
531+
await queryClient.cancelQueries({ queryKey: key })
532+
queryClient.setQueryData(key, [100])
533+
await vi.advanceTimersByTimeAsync(100)
534+
535+
expect(streamFn).not.toHaveBeenCalled()
536+
expect(queryClient.getQueryData(key)).toStrictEqual([100])
537+
unsubscribe()
538+
},
539+
)
540+
541+
it('should keep a custom signal getter lazy', async () => {
542+
const key = createKey()
543+
const controller = new AbortController()
544+
const getSignal = vi.fn<() => AbortSignal>(() => controller.signal)
545+
const queryFn = streamedQuery({
546+
streamFn: () => createAsyncNumberGenerator(1),
547+
})
548+
549+
const result = queryFn({
550+
client: queryClient,
551+
queryKey: key,
552+
meta: undefined,
553+
get signal() {
554+
return getSignal()
555+
},
556+
})
557+
await vi.advanceTimersByTimeAsync(50)
558+
559+
await expect(result).resolves.toStrictEqual([0])
560+
expect(getSignal).not.toHaveBeenCalled()
561+
})
562+
399563
it('should support custom reducer', async () => {
400564
const key = createKey()
401565

‎packages/query-core/src/__tests__/utils.test.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,20 @@ describe('core/utils', () => {
728728
})
729729

730730
describe('addConsumeAwareSignal', () => {
731+
it('should allow page parameters to be passed to URLSearchParams', () => {
732+
const controller = new AbortController()
733+
const context = addConsumeAwareSignal(
734+
{ pageParam: 'cursor' },
735+
() => controller.signal,
736+
vi.fn(),
737+
controller.signal,
738+
)
739+
const { signal, ...params } = context
740+
741+
expect(signal).toBe(controller.signal)
742+
expect(new URLSearchParams(params).toString()).toBe('pageParam=cursor')
743+
})
744+
731745
it('should expose the signal on the query context while preserving its properties', () => {
732746
const controller = new AbortController()
733747
const key = createKey()

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import {
33
addToEnd,
44
addToStart,
55
ensureQueryFn,
6+
fetchSignal,
67
} from './utils'
8+
import type { FetchSignalContext } from './utils'
79
import type { QueryBehavior } from './query'
810
import type {
911
InfiniteData,
@@ -35,11 +37,13 @@ export function infiniteQueryBehavior<TQueryFnData, TError, TData, TPageParam>(
3537

3638
const fetchFn = async () => {
3739
let cancelled = false
38-
const addSignalProperty = (object: unknown) => {
40+
const signalContext: FetchSignalContext = context
41+
const addSignalProperty = (object: object) => {
3942
addConsumeAwareSignal(
4043
object,
4144
() => context.signal,
4245
() => (cancelled = true),
46+
signalContext[fetchSignal],
4347
)
4448
}
4549

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
ensureQueryFn,
3+
fetchSignal,
34
noop,
45
replaceData,
56
resolveQueryValue,
@@ -749,12 +750,15 @@ export class Query<
749750
// Adds an enumerable signal property to the object that
750751
// which sets abortSignalConsumed to true when the signal
751752
// is read.
752-
const addSignalProperty = (object: unknown) => {
753-
Object.defineProperty(object, 'signal', {
754-
enumerable: true,
755-
get: () => {
756-
this.#abortSignalConsumed = true
757-
return abortController.signal
753+
const addSignalProperty = (object: object) => {
754+
Object.defineProperties(object, {
755+
[fetchSignal]: { value: abortController.signal, enumerable: true },
756+
signal: {
757+
enumerable: true,
758+
get: () => {
759+
this.#abortSignalConsumed = true
760+
return abortController.signal
761+
},
758762
},
759763
})
760764
}

0 commit comments

Comments
 (0)