[visitor-plugin-common] fix: memoize fragment field names in getFieldNames to stop exponential blow-up (#10940) - #10944
Draft
eddeee888 wants to merge 3 commits into
Draft
[visitor-plugin-common] fix: memoize fragment field names in getFieldNames to stop exponential blow-up (#10940)#10944eddeee888 wants to merge 3 commits into
eddeee888 wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 23bcbc7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
eddeee888
force-pushed
the
claude/beautiful-fermi-7ybluj
branch
from
September 12, 2026 14:52
f8332e3 to
35a6b5c
Compare
Contributor
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-codegen/visitor-plugin-common |
7.2.6-alpha-20260913024757-23bcbc729bb8c044054d0958237feeea7a45bb2d |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/client-preset |
6.2.0-alpha-20260913024757-23bcbc729bb8c044054d0958237feeea7a45bb2d |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/testing |
5.0.2-alpha-20260913024757-23bcbc729bb8c044054d0958237feeea7a45bb2d |
npm ↗︎ unpkg ↗︎ |
eddeee888
changed the title
…onentially visitor-plugin-common's getFieldNames() recomputes a fragment's field names from scratch on every FRAGMENT_SPREAD instead of computing them once per fragment and reusing the result. When fragments are nested and reused (a "diamond" shape), the work doubles at every level, turning a document with N fragments into O(2^N) work instead of O(N). This is the second root cause reported in #10940 (out-of-memory generating types with near-operation-file + typescript-operations on a schema with deeply nested, widely reused fragments), matching the current source at selection-set-to-object.ts and utils.ts exactly. The added test builds a chain of 20 fragments, each spreading the previous one twice, with a single real field at the bottom. It spies on Set.prototype.add to count how many times that field actually gets walked: today it is 2^20 (1,048,576) instead of staying close to linear in the fragment count. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RmAsddNmmJrhGmTSa1euU4 eddeee888:oss:verify
…l blow-up (#10940) getFieldNames() recomputed a fragment's field names from scratch on every FRAGMENT_SPREAD, even when the same fragment appeared multiple times within the same walk (e.g. fragments nested inside other reused fragments). Since the work doubles at every level of such nesting, a document with N fragments could cost O(2^N) instead of O(N), which is the second root cause reported in #10940. A fragment's own field names don't depend on where it's spread from — only on its own body — so they're now computed once per fragment name (scoped to the lifetime of one top-level getFieldNames() call, via an optional fragmentFieldNamesCache Map threaded through the recursion) and reused, re-prefixing with the current parentName on each reuse. This keeps correctness for a fragment spread at different nesting depths within the same call, while eliminating the redundant re-walk. Fix is scoped to this one function in visitor-plugin-common; it does not touch the run-wide type cache or the cache-key string described as the first root cause in #10940 (left for a follow-up). Verified against the checkpoint test added in the previous commit: `getFieldNames (issue #10940) > does not re-walk a fragment subtree once per spread` now passes (42 Set.add calls instead of 1,048,576 for a 20-level diamond fragment chain). Full visitor-plugin-common suite (51 tests) and downstream consumers - typescript-operations, typescript, near-operation-file-preset, client-preset (440 tests) - all still pass, and `tsc --noEmit` on the whole repo is clean. Related #10940 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RmAsddNmmJrhGmTSa1euU4 eddeee888:oss:fix
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RmAsddNmmJrhGmTSa1euU4
eddeee888
force-pushed
the
claude/beautiful-fermi-7ybluj
branch
from
September 13, 2026 02:47
fe8cb51 to
23bcbc7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes one of the two root causes reported in #10940 (out-of-memory generating types with
near-operation-file+typescript-operationson schemas with deeply nested, widely reused fragments):getFieldNames()invisitor-plugin-commonrecomputed a fragment's field names from scratch on everyFRAGMENT_SPREAD, instead of computing them once per fragment and reusing them. When fragments are nested inside other reused fragments (a "diamond" shape), that work doubles at every level, turning a document with N fragments into O(2^N) work instead of O(N) — the same class of bug as graphql-code-generator-community#752.A fragment's own field names don't depend on where it's spread from, only on its own body, so they can be computed once and reused. This is scoped to
getFieldNames()itself invisitor-plugin-common— it doesn't touch the run-widetypeCacheor the giant joined-string cache key intransformSelectionSet(the other root cause described in #10940), which is left for a follow-up.Related # (issue): #10940
What's changed
packages/plugins/other/visitor-plugin-common/src/utils.ts:getFieldNames()now takes an optionalfragmentFieldNamesCache(aMap<string, ReadonlySet<string>>), threaded through the recursion. On aFRAGMENT_SPREAD, it computes that fragment's field names relative to its own root once, caches them by fragment name, and re-prefixes them with the currentparentNameon every reuse — so a fragment spread at different nesting depths within the same call still gets the correct fully-qualified paths.get-field-names-perf.spec.ts) is unchanged — it's the acceptance criterion for this fix.patchon@graphql-codegen/visitor-plugin-common).Type of change
How Has This Been Tested?
getFieldNames (issue #10940) > does not re-walk a fragment subtree once per spread) now passes: 42Set.addcalls instead of 1,048,576 for a 20-level diamond fragment chain.visitor-plugin-commontest suite (51 tests) passes.typescript-operations,typescript,near-operation-file-preset,client-preset(440 tests total) — all still pass.tsc --noEmiton the whole repo is clean.Test Environment:
@graphql-codegen/visitor-plugin-common: currentmasterChecklist:
Further comments
Root cause #1 from #10940 (the selection-set-to-object.ts cache key built from the full joined field-path list, measured at up to 168MB per key on the reporter's schema) is intentionally out of scope here — it's a separate, additive concern from the exponential re-walk fixed in this PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RmAsddNmmJrhGmTSa1euU4