From 0f0502991ca38b0c3732c36c3ad5611696d722d9 Mon Sep 17 00:00:00 2001 From: Valentin Cocaud Date: Fri, 3 Oct 2025 14:32:31 +0200 Subject: [PATCH 1/5] fix(response-cache): reduce overhead by limiting deep result copy --- packages/plugins/response-cache/src/plugin.ts | 81 +++++++------------ 1 file changed, 28 insertions(+), 53 deletions(-) diff --git a/packages/plugins/response-cache/src/plugin.ts b/packages/plugins/response-cache/src/plugin.ts index 679c4eaeb..25a67a0c9 100644 --- a/packages/plugins/response-cache/src/plugin.ts +++ b/packages/plugins/response-cache/src/plugin.ts @@ -495,14 +495,8 @@ export function useResponseCache = {}> result: ExecutionResult, setResult: (newResult: ExecutionResult) => void, ): void { - let changed = false; if (result.data) { - result = { ...result }; - result.data = removeMetadataFieldsFromResult( - result.data as Record, - onEntity, - ); - changed = true; + collectEntities(result.data as Record, onEntity); } const cacheInstance = cacheFactory(onExecuteParams.args.contextValue); @@ -523,9 +517,6 @@ export function useResponseCache = {}> ); } } - if (changed) { - setResult(result); - } } if (invalidateViaMutation !== false) { @@ -591,7 +582,7 @@ export function useResponseCache = {}> setResult: (newResult: ExecutionResult) => void, ) { if (result.data) { - result.data = removeMetadataFieldsFromResult(result.data, onEntity); + collectEntities(result.data, onEntity); } // we only use the global ttl if no currentTtl has been determined. @@ -656,7 +647,7 @@ function handleAsyncIterableResult = { onNext(payload) { // This is the first result with the initial data payload sent to the client. We use it as the base result if (payload.result.data) { - result.data = payload.result.data; + result.data = deepCopy(payload.result.data); } if (payload.result.errors) { result.errors = payload.result.errors; @@ -669,7 +660,7 @@ function handleAsyncIterableResult = { const { incremental, hasNext } = payload.result; if (incremental) { for (const patch of incremental) { - mergeIncrementalResult({ executionResult: result, incrementalResult: patch }); + mergeIncrementalResult({ executionResult: result, incrementalResult: deepCopy(patch) }); } } @@ -679,32 +670,22 @@ function handleAsyncIterableResult = { } } - const newResult = { ...payload.result }; - - // Handle initial/single result - if (newResult.data) { - newResult.data = removeMetadataFieldsFromResult(newResult.data); + if (payload.result.data) { + collectEntities(payload.result.data); } // Handle Incremental results - if ('hasNext' in newResult && newResult.incremental) { - newResult.incremental = newResult.incremental.map(value => { + if ('hasNext' in payload.result && payload.result.incremental) { + payload.result.incremental = payload.result.incremental.map(value => { if ('items' in value && value.items) { - return { - ...value, - items: removeMetadataFieldsFromResult(value.items), - }; + collectEntities(value.items); } if ('data' in value && value.data) { - return { - ...value, - data: removeMetadataFieldsFromResult(value.data), - }; + collectEntities(value.data); } return value; }); } - payload.setResult(newResult); }, }; } @@ -764,39 +745,30 @@ type OnEntityHandler = ( data: Record, ) => void | Promise; -function removeMetadataFieldsFromResult( - data: Record, - onEntity?: OnEntityHandler, -): Record; -function removeMetadataFieldsFromResult( - data: Array>, - onEntity?: OnEntityHandler, -): Array>; -function removeMetadataFieldsFromResult( - data: null | undefined, - onEntity?: OnEntityHandler, -): null | undefined; -function removeMetadataFieldsFromResult( +function collectEntities(data: Record, onEntity?: OnEntityHandler): void; +function collectEntities(data: Array>, onEntity?: OnEntityHandler): void; +function collectEntities(data: null | undefined, onEntity?: OnEntityHandler): void; +function collectEntities( data: Record | Array> | null | undefined, onEntity?: OnEntityHandler, -): Record | Array | null | undefined { +): void { if (Array.isArray(data)) { - return data.map(record => removeMetadataFieldsFromResult(record, onEntity)); + for (const record of data) { + collectEntities(record, onEntity); + } + return; } if (typeof data !== 'object' || data == null) { - return data; + return; } const dataPrototype = Object.getPrototypeOf(data); if (dataPrototype != null && dataPrototype !== Object.prototype) { - return data; + return; } - // clone the data to avoid mutation - data = { ...data }; - const typename = data.__responseCacheTypeName ?? data.__typename; if (typeof typename === 'string') { const entity: CacheEntityRecord = { typename }; @@ -816,12 +788,15 @@ function removeMetadataFieldsFromResult( for (const key in data) { const value = data[key]; if (Array.isArray(value)) { - data[key] = removeMetadataFieldsFromResult(value, onEntity); + collectEntities(value, onEntity); } if (value !== null && typeof value === 'object') { - data[key] = removeMetadataFieldsFromResult(value as Record, onEntity); + collectEntities(value as Record, onEntity); } } - - return data; } + +const deepCopy = | unknown[] | unknown>(obj: T): T => { + // TODO: find an actual deep copy util + return JSON.parse(JSON.stringify(obj)); +}; From 61804b867059cce11d5fe67f6a2e4c3345adefdf Mon Sep 17 00:00:00 2001 From: Valentin Cocaud Date: Fri, 3 Oct 2025 15:43:06 +0200 Subject: [PATCH 2/5] fix deepcopy --- packages/plugins/response-cache/src/plugin.ts | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/packages/plugins/response-cache/src/plugin.ts b/packages/plugins/response-cache/src/plugin.ts index 25a67a0c9..ac45abaab 100644 --- a/packages/plugins/response-cache/src/plugin.ts +++ b/packages/plugins/response-cache/src/plugin.ts @@ -496,7 +496,7 @@ export function useResponseCache = {}> setResult: (newResult: ExecutionResult) => void, ): void { if (result.data) { - collectEntities(result.data as Record, onEntity); + removeMetadataFieldsFromResult(result.data as Record, onEntity); } const cacheInstance = cacheFactory(onExecuteParams.args.contextValue); @@ -582,7 +582,7 @@ export function useResponseCache = {}> setResult: (newResult: ExecutionResult) => void, ) { if (result.data) { - collectEntities(result.data, onEntity); + removeMetadataFieldsFromResult(result.data, onEntity); } // we only use the global ttl if no currentTtl has been determined. @@ -647,7 +647,7 @@ function handleAsyncIterableResult = { onNext(payload) { // This is the first result with the initial data payload sent to the client. We use it as the base result if (payload.result.data) { - result.data = deepCopy(payload.result.data); + result.data = structuredClone(payload.result.data); } if (payload.result.errors) { result.errors = payload.result.errors; @@ -660,7 +660,10 @@ function handleAsyncIterableResult = { const { incremental, hasNext } = payload.result; if (incremental) { for (const patch of incremental) { - mergeIncrementalResult({ executionResult: result, incrementalResult: deepCopy(patch) }); + mergeIncrementalResult({ + executionResult: result, + incrementalResult: structuredClone(patch), + }); } } @@ -671,17 +674,17 @@ function handleAsyncIterableResult = { } if (payload.result.data) { - collectEntities(payload.result.data); + removeMetadataFieldsFromResult(payload.result.data); } // Handle Incremental results if ('hasNext' in payload.result && payload.result.incremental) { payload.result.incremental = payload.result.incremental.map(value => { if ('items' in value && value.items) { - collectEntities(value.items); + removeMetadataFieldsFromResult(value.items); } if ('data' in value && value.data) { - collectEntities(value.data); + removeMetadataFieldsFromResult(value.data); } return value; }); @@ -745,16 +748,22 @@ type OnEntityHandler = ( data: Record, ) => void | Promise; -function collectEntities(data: Record, onEntity?: OnEntityHandler): void; -function collectEntities(data: Array>, onEntity?: OnEntityHandler): void; -function collectEntities(data: null | undefined, onEntity?: OnEntityHandler): void; -function collectEntities( +function removeMetadataFieldsFromResult( + data: Record, + onEntity?: OnEntityHandler, +): void; +function removeMetadataFieldsFromResult( + data: Array>, + onEntity?: OnEntityHandler, +): void; +function removeMetadataFieldsFromResult(data: null | undefined, onEntity?: OnEntityHandler): void; +function removeMetadataFieldsFromResult( data: Record | Array> | null | undefined, onEntity?: OnEntityHandler, ): void { if (Array.isArray(data)) { for (const record of data) { - collectEntities(record, onEntity); + removeMetadataFieldsFromResult(record, onEntity); } return; } @@ -765,7 +774,11 @@ function collectEntities( const dataPrototype = Object.getPrototypeOf(data); - if (dataPrototype != null && dataPrototype !== Object.prototype) { + // TODO: For some reason, when running in Jest, `structuredClone` result have a weird prototype + // that doesn't equal Object.prototype as it should. + // As a workaround, we can just check that there is no parent prototype, which should be + // the case only when it's the Object prototype + if (dataPrototype != null && Object.getPrototypeOf(dataPrototype) !== null) { return; } @@ -786,17 +799,6 @@ function collectEntities( } for (const key in data) { - const value = data[key]; - if (Array.isArray(value)) { - collectEntities(value, onEntity); - } - if (value !== null && typeof value === 'object') { - collectEntities(value as Record, onEntity); - } + removeMetadataFieldsFromResult(data[key] as any, onEntity); } } - -const deepCopy = | unknown[] | unknown>(obj: T): T => { - // TODO: find an actual deep copy util - return JSON.parse(JSON.stringify(obj)); -}; From 36a0dbc052bc6ce2cdd3879a9514f16366d5ca90 Mon Sep 17 00:00:00 2001 From: Valentin Cocaud Date: Fri, 3 Oct 2025 15:46:15 +0200 Subject: [PATCH 3/5] comment --- packages/plugins/response-cache/src/plugin.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/plugins/response-cache/src/plugin.ts b/packages/plugins/response-cache/src/plugin.ts index ac45abaab..dac6e6307 100644 --- a/packages/plugins/response-cache/src/plugin.ts +++ b/packages/plugins/response-cache/src/plugin.ts @@ -778,7 +778,11 @@ function removeMetadataFieldsFromResult( // that doesn't equal Object.prototype as it should. // As a workaround, we can just check that there is no parent prototype, which should be // the case only when it's the Object prototype + // Rollback once migrated to Bun or vitest + // + // if (dataPrototype != null && dataPrototype !== Object.prototype) { if (dataPrototype != null && Object.getPrototypeOf(dataPrototype) !== null) { + // It is not a plain object, like a Date, don't inspect further return; } From 86f767ff0e4e9332acc9ecf3d39c34a4a5801d29 Mon Sep 17 00:00:00 2001 From: Valentin Cocaud Date: Fri, 3 Oct 2025 16:04:30 +0200 Subject: [PATCH 4/5] changeset --- .changeset/dry-foxes-train.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/dry-foxes-train.md diff --git a/.changeset/dry-foxes-train.md b/.changeset/dry-foxes-train.md new file mode 100644 index 000000000..13786c3a9 --- /dev/null +++ b/.changeset/dry-foxes-train.md @@ -0,0 +1,5 @@ +--- +'@envelop/response-cache': minor +--- + +Reduce overhead From b56597767a1f741fbd6420f60af9abdf439fd4b8 Mon Sep 17 00:00:00 2001 From: Valentin Cocaud Date: Fri, 3 Oct 2025 16:30:24 +0200 Subject: [PATCH 5/5] try moving filtering fields to serialization phase --- packages/plugins/response-cache/src/plugin.ts | 46 +++++++++++-------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/packages/plugins/response-cache/src/plugin.ts b/packages/plugins/response-cache/src/plugin.ts index dac6e6307..30800ee39 100644 --- a/packages/plugins/response-cache/src/plugin.ts +++ b/packages/plugins/response-cache/src/plugin.ts @@ -25,6 +25,7 @@ import { OnExecuteHookResult, Plugin, } from '@envelop/core'; +import { ExecutionResultWithSerializer } from '@envelop/graphql-jit'; import { getDirective, MapperKind, @@ -496,7 +497,8 @@ export function useResponseCache = {}> setResult: (newResult: ExecutionResult) => void, ): void { if (result.data) { - removeMetadataFieldsFromResult(result.data as Record, onEntity); + collectEntities(result.data as Record, onEntity); + setStringifyWithoutMetadata(result); } const cacheInstance = cacheFactory(onExecuteParams.args.contextValue); @@ -582,7 +584,8 @@ export function useResponseCache = {}> setResult: (newResult: ExecutionResult) => void, ) { if (result.data) { - removeMetadataFieldsFromResult(result.data, onEntity); + collectEntities(result.data, onEntity); + setStringifyWithoutMetadata(result); } // we only use the global ttl if no currentTtl has been determined. @@ -674,21 +677,23 @@ function handleAsyncIterableResult = { } if (payload.result.data) { - removeMetadataFieldsFromResult(payload.result.data); + collectEntities(payload.result.data); } // Handle Incremental results if ('hasNext' in payload.result && payload.result.incremental) { payload.result.incremental = payload.result.incremental.map(value => { if ('items' in value && value.items) { - removeMetadataFieldsFromResult(value.items); + collectEntities(value.items); } if ('data' in value && value.data) { - removeMetadataFieldsFromResult(value.data); + collectEntities(value.data); } return value; }); } + + setStringifyWithoutMetadata(result); }, }; } @@ -748,22 +753,16 @@ type OnEntityHandler = ( data: Record, ) => void | Promise; -function removeMetadataFieldsFromResult( - data: Record, - onEntity?: OnEntityHandler, -): void; -function removeMetadataFieldsFromResult( - data: Array>, - onEntity?: OnEntityHandler, -): void; -function removeMetadataFieldsFromResult(data: null | undefined, onEntity?: OnEntityHandler): void; -function removeMetadataFieldsFromResult( +function collectEntities(data: Record, onEntity?: OnEntityHandler): void; +function collectEntities(data: Array>, onEntity?: OnEntityHandler): void; +function collectEntities(data: null | undefined, onEntity?: OnEntityHandler): void; +function collectEntities( data: Record | Array> | null | undefined, onEntity?: OnEntityHandler, ): void { if (Array.isArray(data)) { for (const record of data) { - removeMetadataFieldsFromResult(record, onEntity); + collectEntities(record, onEntity); } return; } @@ -789,20 +788,29 @@ function removeMetadataFieldsFromResult( const typename = data.__responseCacheTypeName ?? data.__typename; if (typeof typename === 'string') { const entity: CacheEntityRecord = { typename }; - delete data.__responseCacheTypeName; if ( data.__responseCacheId && (typeof data.__responseCacheId === 'string' || typeof data.__responseCacheId === 'number') ) { entity.id = data.__responseCacheId; - delete data.__responseCacheId; } onEntity?.(entity, data); } for (const key in data) { - removeMetadataFieldsFromResult(data[key] as any, onEntity); + collectEntities(data[key] as any, onEntity); } } + +const setStringifyWithoutMetadata = (result: ExecutionResultWithSerializer) => { + result.stringify = stringifyWithoutMetadata; + return result; +}; + +const stringifyWithoutMetadata: ExecutionResultWithSerializer['stringify'] = result => { + return JSON.stringify(result, (key: string, value: unknown) => + key === '__responseCacheId' || key === '__responseCacheTypeName' ? undefined : value, + ); +};