fix(visitor-plugin-common): handles more cases in conditional spread of a fragment

[vkbansal-rubrik]

Description

Fixes TypeError: Unexpected type. thrown from
@graphql-codegen/visitor-plugin-common's selection-set-to-object.ts
buildSelectionSet when a fragment is spread with a conditional
directive (@include / @skip / @defer) and the spread fragment's
top-level selection set contains an InlineFragment (... on X { ... })
or another FragmentSpread (...Other).

Root cause

buildFragmentSpreadsUsage only wraps top-level FIELD nodes from the
spread fragment with fragmentDirectives; INLINE_FRAGMENT and
FRAGMENT_SPREAD nodes pass through as raw AST. For a conditional
spread, _buildGroupedSelections then inlines those raw nodes into
flattenedSelectionNodes and feeds the result to buildSelectionSet,
which only handles FIELD / DIRECTIVE for nodes with a kind —
anything else hits throw new TypeError('Unexpected type.').

The non-conditional spread path already does the right thing: it routes
FragmentSpreadUsage.selectionNodes through flattenSelectionSet
recursively (which expands inline fragments and nested spreads per
type) before consuming them.

Fix

In _buildGroupedSelections, after a conditional FragmentSpreadUsage
is inlined into flattenedSelectionNodes, partition the result and
route top-level INLINE_FRAGMENT / FRAGMENT_SPREAD nodes through the
existing flattenSelectionSet pipeline before handing the FIELD-only
merged set to buildSelectionSet. buildSelectionSet's contract — that
raw AST nodes are FIELD or DIRECTIVE — is preserved, and FIELD-only
callers are byte-for-byte unaffected.

Related # (issue) (replace with the issue number after filing the
bug report)

---

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

---

Screenshots/Sandbox (if appropriate/relevant):

Minimal reproduction repo (follows the layout of
graphql-code-generator-issue-sandbox-template):

https://github.com/vkbansal-rubrik/codegen-conditional-spread-bug

$ git clone https://github.com/vkbansal-rubrik/codegen-conditional-spread-bug.git
$ cd codegen-conditional-spread-bug
$ pnpm install
$ pnpm generate

❯ Generate
✖ Generate [FAILED: Unexpected type.]

With this PR applied, pnpm generate completes and emits a valid
LibraryQuery type whose featured field is a union of the
type-narrowed members plus an empty-object variant covering
$includeFeatured = false.

---

How Has This Been Tested?

Two new test cases added to
packages/plugins/typescript/operations/tests/ts-documents.skip-include-directives.spec.ts,
both inside the existing TypeScript Operations Plugin - @include directives
describe block. Each uses a generic Book | Magazine (Publication) schema
with a Library parent — mirroring the regression in different shapes:

• Test A — top-level inline fragments:
  handles conditional spread of a fragment whose top-level selections contain inline fragments.
  The spread fragment contains ... on Book { ... } and
  ... on Magazine { ... } at its top level and is conditionally spread
  via @include(if: $includeFeatured). Without the fix this throws
  Unexpected type.; with the fix the generated TypeScript compiles
  and exposes the union members.
• Test B — top-level fragment spreads:
  handles conditional spread of a fragment whose top-level selections are fragment spreads.
  Same shape as Test A, but the top-level inline fragments are
  replaced with named fragment spreads (...BookFragment /
  ...MagazineFragment). Demonstrates that converting inline
  fragments to named ones is not a sufficient workaround under
  v6 — the same throw fires — and verifies the fix covers both
  shapes.

Manual verification matrix

Before the fix:

× handles conditional spread of a fragment whose top-level selections contain inline fragments
× handles conditional spread of a fragment whose top-level selections are fragment spreads
  TypeError: Unexpected type.
      at SelectionSetToObject.buildSelectionSet selection-set-to-object.ts:805

After the fix:

Suite	Result
New tests (the two added in this PR)	2/2 pass
ts-documents.skip-include-directives.spec.ts (whole file)	20/20 pass
@graphql-codegen/typescript-operations (full suite)	236/237 pass — the 1 remaining failure (imports external custom scalar in shared type file when said scalar is used in relevant Input) also fails on master without this patch, so it is unrelated to this change.
@graphql-codegen/visitor-plugin-common (full suite)	50/50 pass

Test Environment:

• OS: macOS (Darwin 25.5.0)
• @graphql-codegen/...:
  • @graphql-codegen/visitor-plugin-common (patched here)
  • @graphql-codegen/typescript-operations@6.0.2 (new tests here)
  • @graphql-codegen/cli@7.0.0
• NodeJS: 24.13.0 (also reproduced on 20.x and 22.x)
• graphql: 16.14.0 (also reproduces on 15.8.0)

---

Checklist:

• I have followed the
  CONTRIBUTING doc and the
  style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas — the new
  partition/re-flatten block in _buildGroupedSelections carries an inline comment
  explaining why it's needed (the buildSelectionSet contract).
• I have made corresponding changes to the documentation — N/A (no public API change).
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules — N/A
  (single-package change inside visitor-plugin-common; consumers pick it up via the
  bumped version).

---

Further comments

Why this approach over alternatives

Two alternatives I considered and rejected:

1. Expand non-FIELD top-level selections inside buildFragmentSpreadsUsage —
   recursively flatten any top-level INLINE_FRAGMENT / FRAGMENT_SPREAD there, so
   FragmentSpreadUsage.selectionNodes always contains FIELD-shaped entries.
   Cleaner contract for that type, but the change is significantly larger and touches
   the non-conditional path (which is already correct via the recursive
   flattenSelectionSet call in buildSelectionSet's fragment-spread-usage branch).
   I chose to keep the fix surgical to the broken branch.

2. Teach buildSelectionSet to handle INLINE_FRAGMENT / FRAGMENT_SPREAD kinds
   directly at the throw site — would mean duplicating the per-type narrowing logic
   that flattenSelectionSet + _collectInlineFragments already implement. Routing
   through the existing pipeline reuses that logic instead.

Edge case for maintainer review

@include / @skip on an inline fragment inside a conditionally-spread fragment
(e.g. ... on X @include(if: $x) { ... } at the top level of a fragment spread with
its own @include) currently goes through flattenSelectionSet's
inlineFragmentConditionalSelections path, so those fields end up in
selectionNodesByTypeNameConditional, not in the nestedByType map this fix reads.
That nested-conditional case isn't exercised by the new tests or the linked repro and
isn't fixed here. Happy to extend the fix if you consider it in scope.

Workaround documented in the repro

Until this lands, users can restructure their fragments so the top-level selection
set contains only FIELD nodes — i.e., split type-narrowing inline fragments into
per-concrete-type helper fragments and spread each helper directly at the consumer.
The repro README walks through this.

[changeset-bot]

🦋 Changeset detected

Latest commit: 60926ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@graphql-codegen/visitor-plugin-common	Patch
@graphql-codegen/typescript-operations	Patch

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

[rwe]

This patch helped me a bit, thank you @vkbansal-rubrik. I'd also run into the error when using conditional inline fragments, and the bare TypeError with no context was pretty rough…

After this patch unblocked me there appears to be a related issue, which is that multiple conditional fragments can be given the same exported names (e.g. export type FooFragment_Bar_thing_Thing) when they overlap and using exportFragmentSpreadSubTypes. This didn't happen before the big v7 rewrite. It's hard to untangle all the breakages from the removal of "preResolveTypes", but I could plausibly see it be related to this, e.g. in how those conditional fragments are internally named and counted, but I'm not sure. Just a note!

[eddeee888]

Thanks @vkbansal-rubrik !
I've added snapshot tests, will release now

[eddeee888]

After this patch unblocked me there appears to be a related issue, which is that multiple conditional fragments can be given the same exported names (e.g. export type FooFragment_Bar_thing_Thing) when they overlap and using exportFragmentSpreadSubTypes. This didn't happen before the big v7 rewrite. It's hard to untangle all the breakages from the removal of "preResolveTypes", but I could plausibly see it be related to this, e.g. in how those conditional fragments are internally named and counted, but I'm not sure. Just a note!

Hi @rwe, could you please open another issue to help me track?

[eddeee888]

Released in https://github.com/dotansimha/graphql-code-generator/releases/tag/release-1782309865227