From a9e281bb98f24c4cf7e8ae145db7e4560d04c0db Mon Sep 17 00:00:00 2001 From: Bohdan Yefimenko Date: Tue, 27 Aug 2024 17:42:08 +0300 Subject: [PATCH 1/8] fix(query-core): add test case for not serializable data --- .../query-core/src/__tests__/query.test.tsx | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 1f76177467..5b8b9c097b 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -965,4 +965,44 @@ describe('query', () => { await sleep(60) // let it resolve expect(spy).toHaveBeenCalledWith('1 - 2') }) + + it('should have an error status when queryFn data is not serializable', async () => { + const key = queryKey() + + const queryFn = vi.fn() + + queryFn.mockImplementation(async () => { + await sleep(1000) + + 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 }) + + const query = queryCache.find({ queryKey: key })! + + expect(queryFn).toHaveBeenCalledTimes(1) + + // NOTE: need second query fetch, cause for first fetch 'prevData' for structural sharing is 'undefined' + await query.fetch() + + expect(query.state.status).toBe('error') + }) }) From b04bf8284bbacdaf80153802c41048514e36044f Mon Sep 17 00:00:00 2001 From: Bohdan Yefimenko Date: Tue, 27 Aug 2024 17:43:26 +0300 Subject: [PATCH 2/8] fix(query-core): handle `setData` error --- packages/query-core/src/query.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index b51ff6b5be..db29cc6ae4 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -499,7 +499,11 @@ export class Query< return } - this.setData(data) + try { + this.setData(data) + } catch (error) { + onError(error as TError) + } // Notify cache callback this.#cache.config.onSuccess?.(data, this as Query) From 02f6fa57892a2d2c825e744a8e2b8437be7c3f9e Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Tue, 27 Aug 2024 17:49:14 +0200 Subject: [PATCH 3/8] Update packages/query-core/src/__tests__/query.test.tsx --- packages/query-core/src/__tests__/query.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 5b8b9c097b..a3f9e61a58 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -972,7 +972,7 @@ describe('query', () => { const queryFn = vi.fn() queryFn.mockImplementation(async () => { - await sleep(1000) + await sleep(10) const data: Array<{ id: number From 18ae646f220348196f526afdb5e3e78bf8bc31a0 Mon Sep 17 00:00:00 2001 From: Bohdan Yefimenko Date: Tue, 27 Aug 2024 18:56:10 +0300 Subject: [PATCH 4/8] fix(query-core): add `return` statement --- packages/query-core/src/query.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/query-core/src/query.ts b/packages/query-core/src/query.ts index db29cc6ae4..1160b1acfd 100644 --- a/packages/query-core/src/query.ts +++ b/packages/query-core/src/query.ts @@ -503,6 +503,7 @@ export class Query< this.setData(data) } catch (error) { onError(error as TError) + return } // Notify cache callback From 1ff334ed2e48f96581679af3a7cc63f3fce6198c Mon Sep 17 00:00:00 2001 From: Bohdan Yefimenko Date: Tue, 27 Aug 2024 19:36:46 +0300 Subject: [PATCH 5/8] fix(query-core): throw the error in case when data is not serializable for non production env --- .../query-core/src/__tests__/query.test.tsx | 3 -- packages/query-core/src/utils.ts | 9 +++++ packages/query-core/tsconfig.vitest-temp.json | 39 +++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 packages/query-core/tsconfig.vitest-temp.json diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index a3f9e61a58..5b20947a5c 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -1000,9 +1000,6 @@ describe('query', () => { expect(queryFn).toHaveBeenCalledTimes(1) - // NOTE: need second query fetch, cause for first fetch 'prevData' for structural sharing is 'undefined' - await query.fetch() - expect(query.state.status).toBe('error') }) }) diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index f84fdcd775..e9a5efad2c 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -246,6 +246,15 @@ export function replaceEqualDeep(a: any, b: any): any { return a } + if (process.env.NODE_ENV !== 'production') { + try { + JSON.stringify(a) + JSON.stringify(b) + } catch { + throw new Error('Values are not serializable') + } + } + const array = isPlainArray(a) && isPlainArray(b) if (array || (isPlainObject(a) && isPlainObject(b))) { diff --git a/packages/query-core/tsconfig.vitest-temp.json b/packages/query-core/tsconfig.vitest-temp.json new file mode 100644 index 0000000000..376df79516 --- /dev/null +++ b/packages/query-core/tsconfig.vitest-temp.json @@ -0,0 +1,39 @@ +{ + "$schema": "https://json.schemastore.org/tsconfig", + "compilerOptions": { + "allowJs": true, + "allowSyntheticDefaultImports": true, + "allowUnreachableCode": false, + "allowUnusedLabels": false, + "checkJs": true, + "declaration": true, + "esModuleInterop": true, + "forceConsistentCasingInFileNames": true, + "isolatedModules": true, + "lib": [ + "DOM", + "DOM.Iterable", + "ES2022" + ], + "module": "ES2022", + "moduleResolution": "Bundler", + "noEmit": true, + "noImplicitReturns": true, + "noUncheckedIndexedAccess": true, + "noUnusedLocals": true, + "noUnusedParameters": true, + "resolveJsonModule": true, + "skipLibCheck": true, + "strict": true, + "target": "ES2020", + "emitDeclarationOnly": false, + "incremental": true, + "tsBuildInfoFile": "/Users/bohdanyefimenko/projects/query/node_modules/.pnpm/vitest@2.0.5_@types+node@20.14.13_jsdom@24.1.1_less@4.2.0_sass@1.77.8_terser@5.31.3/node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo" + }, + "include": [ + "src", + "eslint.config.js", + "tsup.config.js", + "vite.config.ts" + ] +} \ No newline at end of file From b0627f5917c26efac633b53234165a1d08376a2d Mon Sep 17 00:00:00 2001 From: Bohdan Yefimenko Date: Tue, 27 Aug 2024 19:38:19 +0300 Subject: [PATCH 6/8] fix(query-core): delete packages/query-core/tsconfig.vitest-temp.json --- packages/query-core/tsconfig.vitest-temp.json | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 packages/query-core/tsconfig.vitest-temp.json diff --git a/packages/query-core/tsconfig.vitest-temp.json b/packages/query-core/tsconfig.vitest-temp.json deleted file mode 100644 index 376df79516..0000000000 --- a/packages/query-core/tsconfig.vitest-temp.json +++ /dev/null @@ -1,39 +0,0 @@ -{ - "$schema": "https://json.schemastore.org/tsconfig", - "compilerOptions": { - "allowJs": true, - "allowSyntheticDefaultImports": true, - "allowUnreachableCode": false, - "allowUnusedLabels": false, - "checkJs": true, - "declaration": true, - "esModuleInterop": true, - "forceConsistentCasingInFileNames": true, - "isolatedModules": true, - "lib": [ - "DOM", - "DOM.Iterable", - "ES2022" - ], - "module": "ES2022", - "moduleResolution": "Bundler", - "noEmit": true, - "noImplicitReturns": true, - "noUncheckedIndexedAccess": true, - "noUnusedLocals": true, - "noUnusedParameters": true, - "resolveJsonModule": true, - "skipLibCheck": true, - "strict": true, - "target": "ES2020", - "emitDeclarationOnly": false, - "incremental": true, - "tsBuildInfoFile": "/Users/bohdanyefimenko/projects/query/node_modules/.pnpm/vitest@2.0.5_@types+node@20.14.13_jsdom@24.1.1_less@4.2.0_sass@1.77.8_terser@5.31.3/node_modules/vitest/dist/chunks/tsconfig.tmp.tsbuildinfo" - }, - "include": [ - "src", - "eslint.config.js", - "tsup.config.js", - "vite.config.ts" - ] -} \ No newline at end of file From e7331741a0ed06b98aa4265de5c403502fae0b7c Mon Sep 17 00:00:00 2001 From: Bohdan Yefimenko Date: Wed, 28 Aug 2024 22:41:45 +0300 Subject: [PATCH 7/8] fix(query-core): lift serializability check and add console.error --- .../query-core/src/__tests__/query.test.tsx | 10 +++++++++ packages/query-core/src/utils.ts | 22 +++++++++++-------- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 5b20947a5c..812045d043 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -967,6 +967,10 @@ describe('query', () => { }) it('should have an error status when queryFn data is not serializable', async () => { + const consoleMock = vi.spyOn(console, 'error') + + consoleMock.mockImplementation(() => undefined) + const key = queryKey() const queryFn = vi.fn() @@ -1000,6 +1004,12 @@ describe('query', () => { expect(queryFn).toHaveBeenCalledTimes(1) + expect(consoleMock).toHaveBeenCalledWith( + expect.stringContaining('Data is not serializable'), + ) + expect(query.state.status).toBe('error') + + consoleMock.mockRestore() }) }) diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index e9a5efad2c..3f20bbcadf 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -246,15 +246,6 @@ export function replaceEqualDeep(a: any, b: any): any { return a } - if (process.env.NODE_ENV !== 'production') { - try { - JSON.stringify(a) - JSON.stringify(b) - } catch { - throw new Error('Values are not serializable') - } - } - const array = isPlainArray(a) && isPlainArray(b) if (array || (isPlainObject(a) && isPlainObject(b))) { @@ -362,6 +353,19 @@ 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( + `Data is not serializable. [${options.queryHash}]: ${error}; The error will be redacted in production builds`, + ) + + throw new Error('Data is not serializable') + } + } + // Structurally share data between prev and new data if needed return replaceEqualDeep(prevData, data) } From 82d75b75dbd6f19a8ec2e7646a4c7e59680c0430 Mon Sep 17 00:00:00 2001 From: Dominik Dorfmeister Date: Fri, 30 Aug 2024 12:08:34 +0200 Subject: [PATCH 8/8] chore: message --- packages/query-core/src/__tests__/query.test.tsx | 4 +++- packages/query-core/src/utils.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/query-core/src/__tests__/query.test.tsx b/packages/query-core/src/__tests__/query.test.tsx index 812045d043..dc87e16fac 100644 --- a/packages/query-core/src/__tests__/query.test.tsx +++ b/packages/query-core/src/__tests__/query.test.tsx @@ -1005,7 +1005,9 @@ describe('query', () => { expect(queryFn).toHaveBeenCalledTimes(1) expect(consoleMock).toHaveBeenCalledWith( - expect.stringContaining('Data is not serializable'), + expect.stringContaining( + 'StructuralSharing requires data to be JSON serializable', + ), ) expect(query.state.status).toBe('error') diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts index 3f20bbcadf..a33671c202 100644 --- a/packages/query-core/src/utils.ts +++ b/packages/query-core/src/utils.ts @@ -359,7 +359,7 @@ export function replaceData< JSON.stringify(data) } catch (error) { console.error( - `Data is not serializable. [${options.queryHash}]: ${error}; The error will be redacted in production builds`, + `StructuralSharing 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('Data is not serializable')