From e4338850610512b3f97cca44ce1259e17b7ff80f Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Thu, 14 Apr 2022 18:13:55 +0200 Subject: [PATCH 1/4] fix(infiniteQuery): do not consume AbortSignal unless user has consumed it calling context.signal?.addEventListener did consume the signal --- src/core/infiniteQueryBehavior.ts | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/core/infiniteQueryBehavior.ts b/src/core/infiniteQueryBehavior.ts index 3fe93a0377..ebe073cd90 100644 --- a/src/core/infiniteQueryBehavior.ts +++ b/src/core/infiniteQueryBehavior.ts @@ -6,7 +6,6 @@ import type { QueryOptions, RefetchQueryFilters, } from './types' -import { getAbortController } from './utils' export function infiniteQueryBehavior< TQueryFnData, @@ -24,11 +23,25 @@ export function infiniteQueryBehavior< const isFetchingPreviousPage = fetchMore?.direction === 'backward' const oldPages = context.state.data?.pages || [] const oldPageParams = context.state.data?.pageParams || [] - const abortController = getAbortController() - const abortSignal = abortController?.signal let newPageParams = oldPageParams let cancelled = false + const addSignalProperty = (object: unknown) => { + Object.defineProperty(object, 'signal', { + enumerable: true, + get: () => { + if (context.signal?.aborted) { + cancelled = true; + } else { + context.signal?.addEventListener('abort', () => { + cancelled = true + }) + } + return context.signal; + }, + }) + } + // Get query function const queryFn = context.options.queryFn || (() => Promise.reject('Missing queryFn')) @@ -62,11 +75,12 @@ export function infiniteQueryBehavior< const queryFnContext: QueryFunctionContext = { queryKey: context.queryKey, - signal: abortSignal, pageParam: param, meta: context.meta, } + addSignalProperty(queryFnContext); + const queryFnResult = queryFn(queryFnContext) const promise = Promise.resolve(queryFnResult).then(page => @@ -143,11 +157,6 @@ export function infiniteQueryBehavior< pageParams: newPageParams, })) - context.signal?.addEventListener('abort', () => { - cancelled = true - abortController?.abort() - }) - return finalPromise } }, From ca50839d99b79f851cf3aa3f0c8711b3dcba103b Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Thu, 14 Apr 2022 19:06:37 +0200 Subject: [PATCH 2/4] fix(infiniteQuery): do not consume AbortSignal unless user has consumed it fix formatting --- src/core/infiniteQueryBehavior.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/infiniteQueryBehavior.ts b/src/core/infiniteQueryBehavior.ts index ebe073cd90..df8f8f9798 100644 --- a/src/core/infiniteQueryBehavior.ts +++ b/src/core/infiniteQueryBehavior.ts @@ -31,13 +31,13 @@ export function infiniteQueryBehavior< enumerable: true, get: () => { if (context.signal?.aborted) { - cancelled = true; + cancelled = true } else { context.signal?.addEventListener('abort', () => { cancelled = true }) } - return context.signal; + return context.signal }, }) } @@ -79,7 +79,7 @@ export function infiniteQueryBehavior< meta: context.meta, } - addSignalProperty(queryFnContext); + addSignalProperty(queryFnContext) const queryFnResult = queryFn(queryFnContext) From 577d2d32cd076720497e7cf412473b8f063c7b59 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 25 May 2022 18:27:47 +0200 Subject: [PATCH 3/4] re-write test to reflect the reality we want to continue fetching pages in the background even if the infinite query unmounts, unless the abort signal has been consumed. That is the documented behaviour, and also what useQuery is doing. --- src/reactjs/tests/useInfiniteQuery.test.tsx | 38 +++++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/reactjs/tests/useInfiniteQuery.test.tsx b/src/reactjs/tests/useInfiniteQuery.test.tsx index 6253707e4b..613cdfbc57 100644 --- a/src/reactjs/tests/useInfiniteQuery.test.tsx +++ b/src/reactjs/tests/useInfiniteQuery.test.tsx @@ -1033,20 +1033,19 @@ describe('useInfiniteQuery', () => { }) }) - it('should stop fetching additional pages when the component is unmounted', async () => { + it('should stop fetching additional pages when the component is unmounted and AbortSignal is consumed', async () => { const key = queryKey() - const states: UseInfiniteQueryResult[] = [] let fetches = 0 - const initialData = { pages: [1, 2, 3, 4], pageParams: [1, 2, 3, 4] } + const initialData = { pages: [1, 2, 3, 4], pageParams: [0, 1, 2, 3] } function List() { - const state = useInfiniteQuery( + useInfiniteQuery( key, - async ({ pageParam }) => { + async ({ pageParam = 0, signal: _ }) => { fetches++ await sleep(50) - return Number(pageParam) + return Number(pageParam) * 5 }, { initialData, @@ -1054,8 +1053,6 @@ describe('useInfiniteQuery', () => { } ) - states.push(state) - return null } @@ -1075,13 +1072,24 @@ describe('useInfiniteQuery', () => { await sleep(300) - expect(states.length).toBe(1) - expect(fetches).toBe(2) - expect(queryClient.getQueryState(key)).toMatchObject({ - data: initialData, - status: 'success', - error: null, - }) + if (typeof AbortSignal === 'function') { + console.log('with abortSignal') + expect(fetches).toBe(2) + expect(queryClient.getQueryState(key)).toMatchObject({ + data: initialData, + status: 'success', + error: null, + }) + } else { + console.log('without abortSignal') + // if AbortSignal is not consumed (not available in node14), fetches should not abort + expect(fetches).toBe(4) + expect(queryClient.getQueryState(key)).toMatchObject({ + data: { pages: [0, 5, 10, 15], pageParams: [0, 1, 2, 3] }, + status: 'success', + error: null, + }) + } }) it('should be able to override the cursor in the fetchNextPage callback', async () => { From 0cec964e3b684b2b059d591c3217e64c46674fa8 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Wed, 25 May 2022 18:48:46 +0200 Subject: [PATCH 4/4] fix test --- src/reactjs/tests/useInfiniteQuery.test.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/reactjs/tests/useInfiniteQuery.test.tsx b/src/reactjs/tests/useInfiniteQuery.test.tsx index 613cdfbc57..21d24958a2 100644 --- a/src/reactjs/tests/useInfiniteQuery.test.tsx +++ b/src/reactjs/tests/useInfiniteQuery.test.tsx @@ -1045,11 +1045,13 @@ describe('useInfiniteQuery', () => { async ({ pageParam = 0, signal: _ }) => { fetches++ await sleep(50) - return Number(pageParam) * 5 + return Number(pageParam) * 10 }, { initialData, - getNextPageParam: lastPage => lastPage + 1, + getNextPageParam: (_, allPages) => { + return allPages.length === 4 ? undefined : allPages.length + }, } ) @@ -1073,7 +1075,6 @@ describe('useInfiniteQuery', () => { await sleep(300) if (typeof AbortSignal === 'function') { - console.log('with abortSignal') expect(fetches).toBe(2) expect(queryClient.getQueryState(key)).toMatchObject({ data: initialData, @@ -1081,11 +1082,10 @@ describe('useInfiniteQuery', () => { error: null, }) } else { - console.log('without abortSignal') - // if AbortSignal is not consumed (not available in node14), fetches should not abort + // if AbortSignal is not consumed, fetches should not abort expect(fetches).toBe(4) expect(queryClient.getQueryState(key)).toMatchObject({ - data: { pages: [0, 5, 10, 15], pageParams: [0, 1, 2, 3] }, + data: { pages: [0, 10, 20, 30], pageParams: [0, 1, 2, 3] }, status: 'success', error: null, })