Skip to content

fix(codegen): recursively resolve nested fragment spreads in generated documents#424

Merged
wax911 merged 2 commits into
developfrom
fix/codegen-recursive-fragment-resolution-423
Jun 24, 2026
Merged

fix(codegen): recursively resolve nested fragment spreads in generated documents#424
wax911 merged 2 commits into
developfrom
fix/codegen-recursive-fragment-resolution-423

Conversation

@wax911

@wax911 wax911 commented Jun 24, 2026

Copy link
Copy Markdown
Member

Description

Fixes #423. The codegen FragmentResolver only inlined fragment references directly referenced from the operation document. When a fragment transitively referenced other fragments (e.g. MediaCoreFragment spreads MediaTitleFragment), those nested spreads were left as unresolved ...FragmentName in the generated document, producing invalid GraphQL requests.

Root Cause

flattenFragments() used collectReferencedNames(document) which only collected fragment spreads from the operation document itself. Then relevantDefinitions was filtered to only those directly-referenced names — excluding any fragments only reachable through nested spreads inside fragment definitions.

Fix (Option A — Recursive Inline)

Replaced the shallow single-pass approach with a 4-phase resolution:

Phase Method What it does
1 computeReachableFragments BFS from operation → fragment definitions → nested definitions, computing full transitive closure. Missing fragment definitions throw at build time.
2 detectCycles DFS with inStack/visited over reachable fragments. Cycles at any depth are rejected.
3 AstTransformer inlining Replaces FragmentSpreadInlineFragment using the full transitive closure for relevantDefinitions. AstTransformer recurses into children automatically. Directives (@include, @skip) from the original spread are preserved on the generated inline fragment.
4 Invariant check AST walk to verify zero FragmentSpread nodes remain (defensive guard).

Tests

19 tests total (6 existing + 13 new):

  • Transitive nested (operation → Parent → Child)
  • Deep transitive chain (3+ levels)
  • Sibling nested fragments (one parent, multiple children)
  • Shared nested fragment across multiple operations
  • Missing nested fragment → build error
  • Cycles through transitive references → build error
  • Variable preservation through transitive inlining
  • Independent operation resolution
  • No-spread operation with available fragments
  • Directive preservation: @include(if: $enabled)
  • Directive preservation: @skip(if: $skip)
  • No-directive spread baseline

Validation

  • All 19 tests pass
  • Full :codegen-core:test suite passes
  • App module codegen regenerated: 8 operation documents produced with zero unresolved named fragment spreads (all inlined to ... on TypeName { fields })
  • No changes to :runtime, :api, or any other module

Notes

  • The .slim/deepwork/issue-423-progress.md file tracks the deepwork session; feel free to remove if not part of project conventions.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (Improve existing functionality)

@codacy-production

codacy-production Bot commented Jun 24, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 26 complexity · 8 duplication

Metric Results
Complexity 26
Duplication 8

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

wax911 commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

One remaining blocker before merge: directive preservation during inline replacement.

The current code replaces a named fragment spread with an inline fragment using only the fragment definition type condition and selection set. That appears to drop directives attached to the original spread.

Example: a spread using @include(if: $enabled) should become an inline fragment that still has @include(if: $enabled). Otherwise the generated document changes query semantics.

Please carry the original spread directives onto the generated inline fragment and add a regression test for @include or @skip on a named fragment spread.

The rest of the PR looks aligned with #423: transitive reachability, missing nested fragment failure, cycle detection, sibling fragments, shared fragments, variable preservation, and independent operation resolution are covered.

@wax911

wax911 commented Jun 24, 2026

Copy link
Copy Markdown
Member Author

Addressed the directive preservation review comment:

  • Fix: The InlineFragment builder now carries node.directives from the original FragmentSpread onto the generated InlineFragment. Spreads using @include(if: \$enabled) or @skip(if: \$skip) will retain those directives after inlining.
  • Tests added: 3 new regression tests — @include preservation, @skip preservation, and a no-directive spread baseline.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto approved automated PR

@wax911 wax911 merged commit 4bd29e5 into develop Jun 24, 2026
11 checks passed
@wax911 wax911 deleted the fix/codegen-recursive-fragment-resolution-423 branch June 24, 2026 21:02
wax911 added a commit that referenced this pull request Jun 25, 2026
* fix(codegen): iteratively inline nested fragment spreads

AstTransformer does not recursively visit children of nodes inserted
via TreeTransformerUtil.changeNode, so a single transformation pass
leaves nested fragment spreads (e.g. ...MediaTitleFragment inside
...MediaCoreFragment) unresolved in the generated document.

Replace the single-pass approach with an iterative loop that re-parses
and re-transforms until no FragmentSpread nodes remain. The defensive
invariant check is now the loop exit condition rather than a
post-hoc assertion that could throw on deeply nested fragments.

Follow-up to #424.

* chore(ide): update Gradle project modules and Kotlin version

Add all composite-build and subproject modules to the IDE Gradle
project settings so they are recognised by the IDE. Update the
Kotlin compiler version from 2.0.21 to 2.4.0 to match the
project toolchain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(codegen): recursively resolve nested fragment spreads in generated documents

1 participant