Skip to content

fix(validation): keep argument defaults inside fragments - #290

Merged
Cito merged 1 commit into
graphql-python:mainfrom
rcybulski1122012:fix-argument-defaults-inside-fragments
Oct 27, 2026
Merged

Cito merged 1 commit into
graphql-python:mainfrom
rcybulski1122012:fix-argument-defaults-inside-fragments

Conversation

@rcybulski1122012

@rcybulski1122012 rcybulski1122012 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Variable usages inside a fragment always got an undefined default value, so VariablesInAllowedPositionRule rejected a nullable variable passed through a fragment to a non-null argument with a default, e.g. $intVar: Int used as nonNullFieldWithDefault(arg: $intVar) inside a fragment. GraphQL-core 3.2 accepted such queries.

Use the argument default for every variable usage, as graphql-js does since graphql/graphql-js#4865.

Variable usages inside a fragment always got an undefined default value, so
VariablesInAllowedPositionRule rejected a nullable variable passed through a
fragment to a non-null argument with a default, e.g. `$intVar: Int` used as
`nonNullFieldWithDefault(arg: $intVar)` inside a fragment. GraphQL-core 3.2
accepted such queries.

Use the argument default for every variable usage, as graphql-js does since
graphql/graphql-js#4865.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@Cito Cito left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the PR, and for the clear description and the tests!

For context: GraphQL-core tracks the tagged graphql-js releases. The current version is based on graphql-js 17.0.2, as you can see in version_js. The upstream fix (graphql/graphql-js#4865) landed after that tag and hasn't been part of a graphql-js release yet. Porting it was already scheduled as the next step of our upcoming update, which will also bring in the other untagged upstream changes since 17.0.2.

That said, the bug is a real regression in 3.3 compared to 3.2, so I'm glad to take your PR as the port of that commit.

@codspeed

codspeed Bot commented Oct 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing rcybulski1122012:fix-argument-defaults-inside-fragments (b89f249) with main (3b7a4f5)

Open in CodSpeed

@Cito
Cito merged commit 679be84 into graphql-python:main Oct 27, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants