From b003420c86da7544d53fa037bb7468e31009461e Mon Sep 17 00:00:00 2001 From: Jonathan Toung Date: Thu, 21 Nov 2024 13:53:08 -0800 Subject: [PATCH 1/2] fix(QueriesObserver): fix improper sorting in QueriesObserver's #findMatchingObservers method This commit fixes a bug in the QueriesObserver's #findMatchingObservers. The bug is present when there are duplicate unsorted queries, for example, ['A', 'B', 'A']. The bug results in #findMatchingObservers to return the queries in sorted order, or ['A', 'A', 'B'], which results in a mismatch between Array and Array. This bug was introduced in https://github.com/TanStack/query/commit/54995777af37d219f34f5a03d18fbc1abb06b8a5 --- .../src/__tests__/queriesObserver.test.tsx | 63 +++++++++++++++++++ packages/query-core/src/queriesObserver.ts | 18 +----- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/packages/query-core/src/__tests__/queriesObserver.test.tsx b/packages/query-core/src/__tests__/queriesObserver.test.tsx index 2c8afb8610..39fa7f72db 100644 --- a/packages/query-core/src/__tests__/queriesObserver.test.tsx +++ b/packages/query-core/src/__tests__/queriesObserver.test.tsx @@ -264,4 +264,67 @@ describe('queriesObserver', () => { // Clean-up unsubscribe2() }) + + test('should handle duplicate query keys in different positions', async () => { + const key1 = queryKey() + const key2 = queryKey() + const queryFn1 = vi.fn().mockReturnValue(1) + const queryFn2 = vi.fn().mockReturnValue(2) + + const observer = new QueriesObserver(queryClient, [ + { queryKey: key1, queryFn: queryFn1 }, + { queryKey: key2, queryFn: queryFn2 }, + { queryKey: key1, queryFn: queryFn1 }, + ]) + + const results: Array> = [] + results.push(observer.getOptimisticResult([ + { queryKey: key1, queryFn: queryFn1 }, + { queryKey: key2, queryFn: queryFn2 }, + { queryKey: key1, queryFn: queryFn1 }, + ], undefined)[0]) + + const unsubscribe = observer.subscribe((result) => { + results.push(result) + }) + + await sleep(1) + unsubscribe() + + expect(results.length).toBe(6) + expect(results[0]).toMatchObject([ + { status: 'pending', fetchStatus: 'idle', data: undefined }, + { status: 'pending', fetchStatus: 'idle', data: undefined }, + { status: 'pending', fetchStatus: 'idle', data: undefined }, + ]) + expect(results[1]).toMatchObject([ + { status: 'pending', fetchStatus: 'fetching', data: undefined }, + { status: 'pending', fetchStatus: 'idle', data: undefined }, + { status: 'pending', fetchStatus: 'idle', data: undefined }, + ]) + expect(results[2]).toMatchObject([ + { status: 'pending', fetchStatus: 'fetching', data: undefined }, + { status: 'pending', fetchStatus: 'fetching', data: undefined }, + { status: 'pending', fetchStatus: 'idle', data: undefined }, + ]) + expect(results[3]).toMatchObject([ + { status: 'success', fetchStatus: 'idle', data: 1 }, + { status: 'pending', fetchStatus: 'fetching', data: undefined }, + { status: 'pending', fetchStatus: 'idle', data: undefined }, + ]) + expect(results[4]).toMatchObject([ + { status: 'success', fetchStatus: 'idle', data: 1 }, + { status: 'pending', fetchStatus: 'fetching', data: undefined }, + { status: 'success', fetchStatus: 'idle', data: 1 }, + ]) + expect(results[5]).toMatchObject([ + { status: 'success', fetchStatus: 'idle', data: 1 }, + { status: 'success', fetchStatus: 'idle', data: 2 }, + { status: 'success', fetchStatus: 'idle', data: 1 }, + ]) + + // Verify that queryFn1 was only called once despite being used twice + expect(queryFn1).toHaveBeenCalledTimes(1) + expect(queryFn2).toHaveBeenCalledTimes(1) + }) }) diff --git a/packages/query-core/src/queriesObserver.ts b/packages/query-core/src/queriesObserver.ts index 600317ec29..b7433f5f7f 100644 --- a/packages/query-core/src/queriesObserver.ts +++ b/packages/query-core/src/queriesObserver.ts @@ -240,28 +240,14 @@ export class QueriesObserver< observer: match, }) } else { - const existingObserver = this.#observers.find( - (o) => o.options.queryHash === defaultedOptions.queryHash, - ) observers.push({ defaultedQueryOptions: defaultedOptions, - observer: - existingObserver ?? - new QueryObserver(this.#client, defaultedOptions), + observer: new QueryObserver(this.#client, defaultedOptions), }) } }) - return observers.sort((a, b) => { - return ( - queries.findIndex( - (q) => q.queryHash === a.defaultedQueryOptions.queryHash, - ) - - queries.findIndex( - (q) => q.queryHash === b.defaultedQueryOptions.queryHash, - ) - ) - }) + return observers } #onUpdate(observer: QueryObserver, result: QueryObserverResult): void { From de9bc9980f25ae155bba9369f25eb38160e75a6a Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:36:37 +0000 Subject: [PATCH 2/2] ci: apply automated fixes --- .../src/__tests__/queriesObserver.test.tsx | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/query-core/src/__tests__/queriesObserver.test.tsx b/packages/query-core/src/__tests__/queriesObserver.test.tsx index 656a7fc9c9..91317d9fe9 100644 --- a/packages/query-core/src/__tests__/queriesObserver.test.tsx +++ b/packages/query-core/src/__tests__/queriesObserver.test.tsx @@ -278,16 +278,21 @@ describe('queriesObserver', () => { ]) const results: Array> = [] - results.push(observer.getOptimisticResult([ - { queryKey: key1, queryFn: queryFn1 }, - { queryKey: key2, queryFn: queryFn2 }, - { queryKey: key1, queryFn: queryFn1 }, - ], undefined)[0]) - + results.push( + observer.getOptimisticResult( + [ + { queryKey: key1, queryFn: queryFn1 }, + { queryKey: key2, queryFn: queryFn2 }, + { queryKey: key1, queryFn: queryFn1 }, + ], + undefined, + )[0], + ) + const unsubscribe = observer.subscribe((result) => { results.push(result) }) - + await sleep(1) unsubscribe() @@ -322,7 +327,7 @@ describe('queriesObserver', () => { { status: 'success', fetchStatus: 'idle', data: 2 }, { status: 'success', fetchStatus: 'idle', data: 1 }, ]) - + // Verify that queryFn1 was only called once despite being used twice expect(queryFn1).toHaveBeenCalledTimes(1) expect(queryFn2).toHaveBeenCalledTimes(1)