From e5361bcb7ba0c21d94ead07d7b7dfc3d2b11e98c Mon Sep 17 00:00:00 2001 From: Eddy Nguyen Date: Mon, 28 Sep 2026 23:23:09 +1000 Subject: [PATCH] [visitor-plugin-common] fix: build type cache keys from selection sets as written to stop OOM with nested fragments (#10940) (#10982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: reproduce #10940 — type cache keys grow exponentially with nested fragment reuse Synthetic repro: fragments F0..F7 where each spreads the previous one under three fields. With `inlineFragmentTypes: 'combine'` the generated output is ~2.3 KB, but the `typeCache` keys built from the fragment-expanded field paths (`getFieldNames`) reach 180 KB for a single key and 483 KB in total, growing ~3.3x per extra nesting level. On large projects this runs out of memory. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013DCrEAx4J6ze2GV4FinZs5 eddeee888:oss:issue-verify * test: reduce #10940 repro to the minimal config Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013DCrEAx4J6ze2GV4FinZs5 * fix: build type cache keys from selection sets as written, not fragment-expanded paths (#10940) The per-selection-set type cache was keyed by every fragment-expanded field path (`getFieldNames`), so keys grew exponentially with nested fragment reuse and exhausted memory on large projects. Keys now describe the selection set as written (fragment spreads by name, plus directives and inline fragments), built by `getSelectionSetCacheKey` and memoized per node in `selectionSetCacheKeys` (both @internal). `getFieldNames` is removed. The repro test now spies on `selectionSetCacheKeys` instead of `Map.prototype.set`. Co-authored-by: Eddy Nguyen Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013DCrEAx4J6ze2GV4FinZs5 eddeee888:oss:issue-fix --------- Co-authored-by: Claude --- .changeset/nested-fragments-cache-keys.md | 12 ++ .../src/selection-set-to-object.ts | 11 +- .../other/visitor-plugin-common/src/utils.ts | 104 +++++++++--------- ...-documents.nested-fragments-memory.spec.ts | 96 ++++++++++++++++ 4 files changed, 164 insertions(+), 59 deletions(-) create mode 100644 .changeset/nested-fragments-cache-keys.md create mode 100644 packages/plugins/typescript/operations/tests/ts-documents.nested-fragments-memory.spec.ts diff --git a/.changeset/nested-fragments-cache-keys.md b/.changeset/nested-fragments-cache-keys.md new file mode 100644 index 00000000000..9e5ea854618 --- /dev/null +++ b/.changeset/nested-fragments-cache-keys.md @@ -0,0 +1,12 @@ +--- +'@graphql-codegen/visitor-plugin-common': patch +'@graphql-codegen/typescript-operations': patch +--- + +Fix out-of-memory errors with deeply nested, widely reused fragments (#10940). + +The type cache used while generating selection set types was keyed by every fragment-expanded field +path, so its keys grew exponentially with fragment nesting. Keys are now built from the selection +set as written, referencing fragment spreads by name, so they stay linear in the size of the +documents. The exported `getFieldNames` helper from `@graphql-codegen/visitor-plugin-common`, which +built those expanded paths, is removed. diff --git a/packages/plugins/other/visitor-plugin-common/src/selection-set-to-object.ts b/packages/plugins/other/visitor-plugin-common/src/selection-set-to-object.ts index 3af8bb9de17..af79a510a5c 100644 --- a/packages/plugins/other/visitor-plugin-common/src/selection-set-to-object.ts +++ b/packages/plugins/other/visitor-plugin-common/src/selection-set-to-object.ts @@ -44,9 +44,9 @@ import type { import { DeclarationBlock, DeclarationBlockConfig, - getFieldNames, getFieldNodeNameValue, getPossibleTypes, + getSelectionSetCacheKey, hasConditionalDirectives, hasIncrementalDeliveryDirectives, mergeSelectionSets, @@ -1107,12 +1107,7 @@ export class SelectionSetToObject< public transformSelectionSet(fieldName: string) { const possibleTypesList = getPossibleTypes(this._schema, this._parentSchemaType); const possibleTypes = possibleTypesList.map(v => v.name).sort(); - const fieldSelections = [ - ...getFieldNames({ - selections: this._selectionSet.selections, - loadedFragments: this._loadedFragments, - }), - ].sort(); + const selectionSetKey = getSelectionSetCacheKey(this._selectionSet); // Optimization: Do not create new dependentTypes if fragment typename exists in cache // 2-layer cache: LOC => Field Selection Type Combination => cachedTypeString @@ -1120,7 +1115,7 @@ export class SelectionSetToObject< this._processor.typeCache.get(this._selectionSet.loc) ?? new Map(); this._processor.typeCache.set(this._selectionSet.loc, objMap); - const cacheHashKey = `${fieldSelections.join(',')} @ ${possibleTypes.join('|')}`; + const cacheHashKey = `${selectionSetKey} @ ${possibleTypes.join('|')}`; const [cachedTypeString] = objMap.get(cacheHashKey) ?? []; if (cachedTypeString) { // reuse previously generated type, as it is identical diff --git a/packages/plugins/other/visitor-plugin-common/src/utils.ts b/packages/plugins/other/visitor-plugin-common/src/utils.ts index 55e85b6ca65..e08a4b15ed1 100644 --- a/packages/plugins/other/visitor-plugin-common/src/utils.ts +++ b/packages/plugins/other/visitor-plugin-common/src/utils.ts @@ -21,6 +21,7 @@ import { Kind, NamedTypeNode, NameNode, + print, SelectionNode, SelectionSetNode, StringValueNode, @@ -30,12 +31,7 @@ import type { RawConfig } from './base-visitor.js'; import { parseMapper } from './mappers.js'; import { DEFAULT_SCALARS } from './scalars.js'; import type { EnrichedFieldNode } from './selection-set-to-object.js'; -import type { - LoadedFragment, - NormalizedScalarsMap, - ParsedScalarsMap, - ScalarsMap, -} from './types.js'; +import type { NormalizedScalarsMap, ParsedScalarsMap, ScalarsMap } from './types.js'; export const getConfigValue = (value: T | null | undefined, defaultValue: T): T => { if (value === null || value === undefined) { @@ -652,64 +648,70 @@ export function unique( return Object.values(array.reduce((acc, item) => ({ [key(item)]: item, ...acc }), {})); } -function getFullPathFieldName(selection: FieldNode, parentName: string) { - const fullName = - 'alias' in selection && selection.alias - ? `${selection.alias.value}@${selection.name.value}` - : selection.name.value; - return parentName ? `${parentName}.${fullName}` : fullName; -} +/** + * Memoizes `getSelectionSetCacheKey` per selection set node. Entries are dropped once the node's + * document is no longer referenced. + * + * @internal Exported for tests only; not part of the public API. + */ +export const selectionSetCacheKeys = new WeakMap(); -export const getFieldNames = ({ - selections, - fieldNames = new Set(), - parentName = '', - loadedFragments, -}: { - selections: readonly SelectionNode[]; - fieldNames?: Set; - parentName?: string; - loadedFragments: LoadedFragment[]; -}) => { - for (const selection of selections) { +/** + * Builds a cache key describing a selection set as written: fragment spreads are referenced by + * name rather than expanded, so the key stays linear in the size of the document even when + * fragments are deeply nested and widely reused. The parts are sorted so the key does not depend + * on selection order. + * + * Examples: + * - `{ user { id name } }` becomes `user{id,name}` (the inner `{ id name }` becomes `id,name`) + * - `{ id ...UserFields }` becomes `...UserFields,id` (the fragment is referenced by name, not + * expanded) + * - `{ ... on Admin { role } }` becomes `... on Admin{role}` + * - `{ me: user { id ...UserFields @include(if: $withFields) ... on Admin { role } } }` becomes + * `me@user{... on Admin{role},...UserFields @include(if: $withFields),id}` + * + * @internal Not part of the public API. + */ +export function getSelectionSetCacheKey(selectionSet: SelectionSetNode): string { + const cached = selectionSetCacheKeys.get(selectionSet); + if (cached !== undefined) { + return cached; + } + + const printDirectives = (directives: readonly DirectiveNode[] | undefined): string => + directives?.length ? ` ${directives.map(directive => print(directive)).join(' ')}` : ''; + + const parts = new Set(); + for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { - const fieldName = getFullPathFieldName(selection, parentName); - fieldNames.add(fieldName); - if (selection.selectionSet) { - getFieldNames({ - selections: selection.selectionSet.selections, - fieldNames, - parentName: fieldName, - loadedFragments, - }); - } + const name = selection.alias + ? `${selection.alias.value}@${selection.name.value}` + : selection.name.value; + const subKey = selection.selectionSet + ? `{${getSelectionSetCacheKey(selection.selectionSet)}}` + : ''; + parts.add(`${name}${printDirectives(selection.directives)}${subKey}`); break; } case Kind.FRAGMENT_SPREAD: { - getFieldNames({ - selections: loadedFragments - .filter(def => def.name === selection.name.value) - .flatMap(s => s.node.selectionSet.selections), - fieldNames, - parentName, - loadedFragments, - }); + parts.add(`...${selection.name.value}${printDirectives(selection.directives)}`); break; } case Kind.INLINE_FRAGMENT: { - getFieldNames({ - selections: selection.selectionSet.selections, - fieldNames, - parentName, - loadedFragments, - }); + const onType = selection.typeCondition ? ` on ${selection.typeCondition.name.value}` : ''; + parts.add( + `...${onType}${printDirectives(selection.directives)}{${getSelectionSetCacheKey(selection.selectionSet)}}`, + ); break; } } } - return fieldNames; -}; + + const key = [...parts].sort().join(','); + selectionSetCacheKeys.set(selectionSet, key); + return key; +} export const getNodeComment = ( node: FieldDefinitionNode | EnumValueDefinitionNode | InputValueDefinitionNode, diff --git a/packages/plugins/typescript/operations/tests/ts-documents.nested-fragments-memory.spec.ts b/packages/plugins/typescript/operations/tests/ts-documents.nested-fragments-memory.spec.ts new file mode 100644 index 00000000000..2108b261a74 --- /dev/null +++ b/packages/plugins/typescript/operations/tests/ts-documents.nested-fragments-memory.spec.ts @@ -0,0 +1,96 @@ +import { buildSchema, parse, print } from 'graphql'; +import { selectionSetCacheKeys } from '@graphql-codegen/visitor-plugin-common'; +import { plugin } from '../src/index.js'; + +const schema = buildSchema(/* GraphQL */ ` + type Query { + root: Node + } + + type Node { + id: ID! + name: String + a: Node + b: Node + c: Node + } +`); + +// A chain of fragments F0..F(DEPTH-1), where each fragment spreads the previous one under several fields. +// The document as written is linear in DEPTH, but its fragment-expanded tree is 3^DEPTH in size. +const DEPTH = 8; +const buildDocumentSource = () => { + const fragments = [ + /* GraphQL */ ` + fragment F0 on Node { + id + name + } + `, + ]; + for (let i = 1; i < DEPTH; i++) { + fragments.push(/* GraphQL */ ` + fragment F${i} on Node { + id + a { + ...F${i - 1} + } + b { + ...F${i - 1} + } + c { + ...F${i - 1} + } + } + `); + } + return /* GraphQL */ ` + query Root { + root { + ...F${DEPTH - 1} + } + } + ${fragments.join('\n')} + `; +}; + +describe('TypeScript Operations Plugin - deeply nested fragments (issue #10940)', () => { + it('does not retain type cache keys that grow exponentially with nested fragment reuse', async () => { + const document = parse(buildDocumentSource()); + const printedDocumentLength = print(document).length; + + // Spy on the memo behind the type cache keys, so we can measure every key that gets built. + const setSpy = vi.spyOn(selectionSetCacheKeys, 'set'); + + let result: Awaited>; + let cacheKeys: string[]; + try { + // No config needed: each option (including every `inlineFragmentTypes` mode) was measured + // and none changes the cache key sizes; the growth comes from nested fragment reuse alone. + result = await plugin( + schema, + [{ location: 'test-file.ts', document }], + {}, + { outputFile: 'graphql.ts' }, + ); + } finally { + // Read the calls before restoring: `mockRestore()` also clears them. + cacheKeys = setSpy.mock.calls.map(([, key]) => key); + setSpy.mockRestore(); + } + + // Output is produced for both the operation and the outermost fragment. + expect(result.content).toMatch(/export type RootQuery\b/); + expect(result.content).toMatch(new RegExp(`export type F${DEPTH - 1}Fragment\\b`)); + + // In user terms: memory retained by the type cache must scale with the documents as written, + // not with the fragment-expanded tree. Otherwise it grows exponentially with nested fragment + // reuse, and large projects with deeply nested, widely reused fragments run out of memory. + // Guard against the spy silently seeing nothing, which would make the bounds below pass vacuously. + expect(cacheKeys.length).toBeGreaterThan(0); + + const bound = 10 * printedDocumentLength; + expect(Math.max(...cacheKeys.map(key => key.length))).toBeLessThan(bound); + expect(cacheKeys.reduce((total, key) => total + key.length, 0)).toBeLessThan(bound); + }); +});