From 73c1752078930773d153695e6df655873b4c27dc Mon Sep 17 00:00:00 2001 From: jxom Date: Sun, 8 Sep 2024 21:30:44 +1000 Subject: [PATCH 1/6] feat: throw when `replaceDeepEqual` contains circular references --- .../query-core/src/__tests__/query.test.tsx | 44 ++++----- .../query-core/src/__tests__/utils.test.tsx | 14 +++ packages/query-core/src/utils.ts | 90 ++++++++++--------- 3 files changed, 87 insertions(+), 61 deletions(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 0b3d4889d62..e2a17ea8d37 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -975,36 +975,40 @@ describe('query', () => { const queryFn = vi.fn() + const data: Array<{ + id: number + name: string + link: null | { id: number; name: string; link: unknown } + }> = Array.from({ length: 5 }) + .fill(null) + .map((_, index) => ({ + id: index, + name: `name-${index}`, + link: null, + })) + + if (data[0] && data[1]) { + data[0].link = data[1] + data[1].link = data[0] + } + queryFn.mockImplementation(async () => { await sleep(10) - - const data: Array<{ - id: number - name: string - link: null | { id: number; name: string; link: unknown } - }> = Array.from({ length: 5 }) - .fill(null) - .map((_, index) => ({ - id: index, - name: `name-${index}`, - link: null, - })) - - if (data[0] && data[1]) { - data[0].link = data[1] - data[1].link = data[0] - } - return data }) - await queryClient.prefetchQuery({ queryKey: key, queryFn }) + await queryClient.prefetchQuery({ queryKey: key, queryFn, initialData: structuredClone(data) }) + + const query = queryCache.find({ queryKey: key })! expect(queryFn).toHaveBeenCalledTimes(1) + expect(query.state.status).toBe('error') + expect(query.state.error?.message.includes('contains non-serializable data. Error: circular reference detected.')).toBeTruthy() + expect(consoleMock).toHaveBeenCalledWith( expect.stringContaining( - 'StructuralSharing requires data to be JSON serializable', + 'Structural sharing requires data to be JSON serializable', ), ) diff --git a/packages/query-core/src/__tests__/utils.test.tsx b/packages/query-core/src/__tests__/utils.test.tsx index e5ebffe3d2b..aa9597afb3d 100644 --- a/packages/query-core/src/__tests__/utils.test.tsx +++ b/packages/query-core/src/__tests__/utils.test.tsx @@ -172,6 +172,20 @@ describe('core/utils', () => { expect(replaceEqualDeep(object, array)).toBe(array) }) + it('should return the next value when the previous value is a circular reference', () => { + const value: Array<{ foo?: unknown }> = [{}] + value[0]!.foo = value + + const value2: Array<{ foo?: unknown }> = [{}] + value2[0]!.foo = value2 + + expect(() => + replaceEqualDeep(value, value2), + ).toThrowErrorMatchingInlineSnapshot( + `[Error: circular reference detected.]`, + ) + }) + it('should return the previous value when the next value is an equal array', () => { const prev = [1, 2] const next = [1, 2] diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index a8f91a633da..51a9b01e457 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -242,42 +242,54 @@ export function partialMatchKey(a: any, b: any): boolean { */ export function replaceEqualDeep(a: unknown, b: T): T export function replaceEqualDeep(a: any, b: any): any { - if (a === b) { - return a - } + const seen = new WeakSet() - const array = isPlainArray(a) && isPlainArray(b) - - if (array || (isPlainObject(a) && isPlainObject(b))) { - const aItems = array ? a : Object.keys(a) - const aSize = aItems.length - const bItems = array ? b : Object.keys(b) - const bSize = bItems.length - const copy: any = array ? [] : {} - - let equalItems = 0 - - for (let i = 0; i < bSize; i++) { - const key = array ? i : bItems[i] - if ( - ((!array && aItems.includes(key)) || array) && - a[key] === undefined && - b[key] === undefined - ) { - copy[key] = undefined - equalItems++ - } else { - copy[key] = replaceEqualDeep(a[key], b[key]) - if (copy[key] === a[key] && a[key] !== undefined) { + function _replaceEqualDeep(a: any, b: any) { + if (a === b) { + return a + } + + const array = isPlainArray(a) && isPlainArray(b) + + if (array || (isPlainObject(a) && isPlainObject(b))) { + if (seen.has(a)) { + throw new Error('circular reference detected.') + } + seen.add(a) + + const aItems = array ? a : Object.keys(a) + const aSize = aItems.length + const bItems = array ? b : Object.keys(b) + const bSize = bItems.length + const copy: any = array ? [] : {} + + let equalItems = 0 + + for (let i = 0; i < bSize; i++) { + const key = array ? i : bItems[i] + + if ( + ((!array && aItems.includes(key)) || array) && + a[key] === undefined && + b[key] === undefined + ) { + copy[key] = undefined equalItems++ + } else { + copy[key] = _replaceEqualDeep(a[key], b[key]) + if (copy[key] === a[key] && a[key] !== undefined) { + equalItems++ + } } } + + return aSize === bSize && equalItems === aSize ? a : copy } - - return aSize === bSize && equalItems === aSize ? a : copy + + return b } - return b + return _replaceEqualDeep(a, b) } /** @@ -354,19 +366,15 @@ export function replaceData< if (typeof options.structuralSharing === 'function') { return options.structuralSharing(prevData, data) as TData } else if (options.structuralSharing !== false) { - if (process.env.NODE_ENV !== 'production') { - try { - JSON.stringify(prevData) - JSON.stringify(data) - } catch (error) { - console.error( - `StructuralSharing requires data to be JSON serializable. To fix this, turn off structuralSharing or return JSON-serializable data from your queryFn. [${options.queryHash}]: ${error}`, - ) - } + try { + // Structurally share data between prev and new data if needed + return replaceEqualDeep(prevData, data) + } catch (error) { + console.error( + `Structural sharing requires data to be JSON serializable. To fix this, turn off structuralSharing or return JSON-serializable data from your queryFn. [${options.queryHash}]: ${error}`, + ) + throw new Error(`Query hash ${options.queryHash} contains non-serializable data. ${error}`) } - - // Structurally share data between prev and new data if needed - return replaceEqualDeep(prevData, data) } return data } From a610e3fc4c29dbaec8662db533e5534adda5e1f3 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 8 Sep 2024 11:35:23 +0000 Subject: [PATCH 2/6] ci: apply automated fixes --- packages/query-core/src/__tests__/query.test.tsx | 12 ++++++++++-- packages/query-core/src/utils.ts | 16 +++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index e2a17ea8d37..7ff8f30af2c 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -997,14 +997,22 @@ describe('query', () => { return data }) - await queryClient.prefetchQuery({ queryKey: key, queryFn, initialData: structuredClone(data) }) + await queryClient.prefetchQuery({ + queryKey: key, + queryFn, + initialData: structuredClone(data), + }) const query = queryCache.find({ queryKey: key })! expect(queryFn).toHaveBeenCalledTimes(1) expect(query.state.status).toBe('error') - expect(query.state.error?.message.includes('contains non-serializable data. Error: circular reference detected.')).toBeTruthy() + expect( + query.state.error?.message.includes( + 'contains non-serializable data. Error: circular reference detected.', + ), + ).toBeTruthy() expect(consoleMock).toHaveBeenCalledWith( expect.stringContaining( diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index 51a9b01e457..90a544c146a 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -248,9 +248,9 @@ export function replaceEqualDeep(a: any, b: any): any { if (a === b) { return a } - + const array = isPlainArray(a) && isPlainArray(b) - + if (array || (isPlainObject(a) && isPlainObject(b))) { if (seen.has(a)) { throw new Error('circular reference detected.') @@ -262,9 +262,9 @@ export function replaceEqualDeep(a: any, b: any): any { const bItems = array ? b : Object.keys(b) const bSize = bItems.length const copy: any = array ? [] : {} - + let equalItems = 0 - + for (let i = 0; i < bSize; i++) { const key = array ? i : bItems[i] @@ -282,10 +282,10 @@ export function replaceEqualDeep(a: any, b: any): any { } } } - + return aSize === bSize && equalItems === aSize ? a : copy } - + return b } @@ -373,7 +373,9 @@ export function replaceData< console.error( `Structural sharing requires data to be JSON serializable. To fix this, turn off structuralSharing or return JSON-serializable data from your queryFn. [${options.queryHash}]: ${error}`, ) - throw new Error(`Query hash ${options.queryHash} contains non-serializable data. ${error}`) + throw new Error( + `Query hash ${options.queryHash} contains non-serializable data. ${error}`, + ) } } return data From 2bcece2583a5d34bb01c1d91b9821873e009f8a3 Mon Sep 17 00:00:00 2001 From: jxom Date: Sun, 8 Sep 2024 21:35:28 +1000 Subject: [PATCH 3/6] chore: up test --- packages/query-core/src/__tests__/utils.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/utils.test.tsx b/packages/query-core/src/__tests__/utils.test.tsx index aa9597afb3d..c1fcc696087 100644 --- a/packages/query-core/src/__tests__/utils.test.tsx +++ b/packages/query-core/src/__tests__/utils.test.tsx @@ -172,7 +172,7 @@ describe('core/utils', () => { expect(replaceEqualDeep(object, array)).toBe(array) }) - it('should return the next value when the previous value is a circular reference', () => { + it('should throw when the value is a circular reference', () => { const value: Array<{ foo?: unknown }> = [{}] value[0]!.foo = value From 8de55409abeab34f6a779f5c804ef20bd81ba1e8 Mon Sep 17 00:00:00 2001 From: jxom Date: Mon, 9 Sep 2024 06:59:03 +1000 Subject: [PATCH 4/6] chore: review changes --- .../query-core/src/__tests__/query.test.tsx | 4 +- .../query-core/src/__tests__/utils.test.tsx | 14 --- packages/query-core/src/utils.ts | 89 ++++++++----------- 3 files changed, 40 insertions(+), 67 deletions(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 7ff8f30af2c..3efbc56aff5 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -1009,9 +1009,7 @@ describe('query', () => { expect(query.state.status).toBe('error') expect( - query.state.error?.message.includes( - 'contains non-serializable data. Error: circular reference detected.', - ), + query.state.error?.message.includes('Maximum call stack size exceeded'), ).toBeTruthy() expect(consoleMock).toHaveBeenCalledWith( diff --git a/packages/query-core/src/__tests__/utils.test.tsx b/packages/query-core/src/__tests__/utils.test.tsx index c1fcc696087..e5ebffe3d2b 100644 --- a/packages/query-core/src/__tests__/utils.test.tsx +++ b/packages/query-core/src/__tests__/utils.test.tsx @@ -172,20 +172,6 @@ describe('core/utils', () => { expect(replaceEqualDeep(object, array)).toBe(array) }) - it('should throw when the value is a circular reference', () => { - const value: Array<{ foo?: unknown }> = [{}] - value[0]!.foo = value - - const value2: Array<{ foo?: unknown }> = [{}] - value2[0]!.foo = value2 - - expect(() => - replaceEqualDeep(value, value2), - ).toThrowErrorMatchingInlineSnapshot( - `[Error: circular reference detected.]`, - ) - }) - it('should return the previous value when the next value is an equal array', () => { const prev = [1, 2] const next = [1, 2] diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index 90a544c146a..536db0e6a70 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -242,56 +242,45 @@ export function partialMatchKey(a: any, b: any): boolean { */ export function replaceEqualDeep(a: unknown, b: T): T export function replaceEqualDeep(a: any, b: any): any { - const seen = new WeakSet() - - function _replaceEqualDeep(a: any, b: any) { - if (a === b) { - return a - } - - const array = isPlainArray(a) && isPlainArray(b) - - if (array || (isPlainObject(a) && isPlainObject(b))) { - if (seen.has(a)) { - throw new Error('circular reference detected.') - } - seen.add(a) - - const aItems = array ? a : Object.keys(a) - const aSize = aItems.length - const bItems = array ? b : Object.keys(b) - const bSize = bItems.length - const copy: any = array ? [] : {} - - let equalItems = 0 - - for (let i = 0; i < bSize; i++) { - const key = array ? i : bItems[i] + if (a === b) { + return a + } - if ( - ((!array && aItems.includes(key)) || array) && - a[key] === undefined && - b[key] === undefined - ) { - copy[key] = undefined + const array = isPlainArray(a) && isPlainArray(b) + + if (array || (isPlainObject(a) && isPlainObject(b))) { + const aItems = array ? a : Object.keys(a) + const aSize = aItems.length + const bItems = array ? b : Object.keys(b) + const bSize = bItems.length + const copy: any = array ? [] : {} + + let equalItems = 0 + + for (let i = 0; i < bSize; i++) { + const key = array ? i : bItems[i] + if ( + ((!array && aItems.includes(key)) || array) && + a[key] === undefined && + b[key] === undefined + ) { + copy[key] = undefined + equalItems++ + } else { + copy[key] = replaceEqualDeep(a[key], b[key]) + if (copy[key] === a[key] && a[key] !== undefined) { equalItems++ - } else { - copy[key] = _replaceEqualDeep(a[key], b[key]) - if (copy[key] === a[key] && a[key] !== undefined) { - equalItems++ - } } } - - return aSize === bSize && equalItems === aSize ? a : copy } - return b + return aSize === bSize && equalItems === aSize ? a : copy } - return _replaceEqualDeep(a, b) + return b } + /** * Shallow compare objects. */ @@ -366,17 +355,17 @@ export function replaceData< if (typeof options.structuralSharing === 'function') { return options.structuralSharing(prevData, data) as TData } else if (options.structuralSharing !== false) { - try { - // Structurally share data between prev and new data if needed - return replaceEqualDeep(prevData, data) - } catch (error) { - console.error( - `Structural sharing requires data to be JSON serializable. To fix this, turn off structuralSharing or return JSON-serializable data from your queryFn. [${options.queryHash}]: ${error}`, - ) - throw new Error( - `Query hash ${options.queryHash} contains non-serializable data. ${error}`, - ) + if (process.env.NODE_ENV !== 'production') { + try { + replaceEqualDeep(prevData, data) + } catch (error) { + console.error( + `Structural sharing requires data to be JSON serializable. To fix this, turn off structuralSharing or return JSON-serializable data from your queryFn. [${options.queryHash}]: ${error}`, + ) + } } + // Structurally share data between prev and new data if needed + return replaceEqualDeep(prevData, data) } return data } From 93b2d56c30088744ccbb377ebf0b6e92295a2116 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 8 Sep 2024 21:00:26 +0000 Subject: [PATCH 5/6] ci: apply automated fixes --- packages/query-core/src/utils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index 536db0e6a70..a1c8d9f3ef7 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -280,7 +280,6 @@ export function replaceEqualDeep(a: any, b: any): any { return b } - /** * Shallow compare objects. */ From 9fc0c0a517be0725233bf5c45e589fd48788428d Mon Sep 17 00:00:00 2001 From: jxom Date: Mon, 9 Sep 2024 07:36:26 +1000 Subject: [PATCH 6/6] chore: add return --- packages/query-core/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index a1c8d9f3ef7..d5137806400 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -356,7 +356,7 @@ export function replaceData< } else if (options.structuralSharing !== false) { if (process.env.NODE_ENV !== 'production') { try { - replaceEqualDeep(prevData, data) + return replaceEqualDeep(prevData, data) } catch (error) { console.error( `Structural sharing requires data to be JSON serializable. To fix this, turn off structuralSharing or return JSON-serializable data from your queryFn. [${options.queryHash}]: ${error}`,