CODEGEN-940 Fix conditional directive (@skip and @include) incorrectly drop fields in Result when used on Inline Fragment / Fragment Spread#10899
Open
eddeee888 wants to merge 9 commits into
Conversation
🦋 Changeset detectedLatest commit: f6c0103 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
@skip and @include) incorrectly drop fields in main Result when used on Inline Fragment / Fragment Spread@skip and @include) incorrectly drop fields in Result when used on Inline Fragment / Fragment Spread
Contributor
💻 Website PreviewThe latest changes are available as preview in: https://pr-10899.graphql-code-generator.pages.dev |
eddeee888
commented
Jul 19, 2026
|
|
||
|
|
||
| export type UserSkipQuery = { user: { id: string } & { name?: string, niName?: string } & { age?: number, createdAt?: string } & ({ age: number, createdAt: string } | { age?: never, createdAt?: never }) | null }; | ||
| export type UserSkipQuery = { user: { id: string } & { name?: string, niName?: string } & { age?: number, createdAt?: string } & ({ age?: number, createdAt?: string } | { age?: never, createdAt?: never }) | null }; |
Collaborator
Author
There was a problem hiding this comment.
This type change actually doesn't have any effect on the final merged type.
Here are the scenarios at runtime:
- When
@include(if: false), the fields are not available i.e.{ }. This is captured in the{ age?: number, createdAt?: string }group where each field isundefined - When
@include(if: true):
a. the fields are not available if they are deferred
b. the fields are available if they are not deferred. HOWEVER, the fields could beundefinedbecause it's used in conjunction with@skip.
eddeee888
force-pushed
the
codegen-940-fix-conditional-directive-on-fragment
branch
from
July 19, 2026 12:56
347ba2f to
8ed8e78
Compare
eddeee888
commented
Jul 19, 2026
| name: String! | ||
| nickName: String! | ||
| age: Int! | ||
| age: Int |
Collaborator
Author
There was a problem hiding this comment.
Making this nullable so we can see the difference between non-nullable and nullable fields when used with these directives
Contributor
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-codegen/visitor-plugin-common |
7.2.2-alpha-20260720140050-f6c010380b76ecfa068fb6136bba705a20c04536 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-operations |
6.1.2-alpha-20260720140050-f6c010380b76ecfa068fb6136bba705a20c04536 |
npm ↗︎ unpkg ↗︎ |
eddeee888
marked this pull request as ready for review
July 20, 2026 13:59
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
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
TODO:
Related #10881
Type of change
Please delete options that are not relevant.
How Has This Been Tested?