Fragment arguments/variables (syntax/validation/execution) #1081

JoviDeCroock · 2024-02-16T07:29:35Z

This spec contains amendments to #1010, a diffed view is available at mjmahone#3.

These amendments are made from comments on the implementation PR and alterations from the new implementation

Changing fragment-key to just be fragment-name Fragment Arguments Reference Implementation graphql-js#3835 (comment)
Removing the substitution logic and going closer to the coerce logic we use in the general flow

In general the biggest changes are that we introduce fragmentVariableValues which will be on the groupedFieldSet, these are derived from the arguments in scope of the fragmentDefinition where this field is used.

We introduce localFragmentVariables which as we are traversing down fragment-spreads are a coerced set of variables i.e.

query($b: String!, $c: String!) {
  field3(b: $b) ## uses b from variable-values
  ...A(a: "A", b: "B")
}

fragment A($a: String!, $b: String!) on X {
  field(a: $a) ## uses $a from localVariableValues
  field2(c: $c) ## we have access to variableValues so we take c from there
  ...B(b: $b) ## uses $b from localVariableValues
}

fragment B($b: String!) on X {
  ## This fragment has access to $b and $c but not to $a
  field4(b: $b) ## uses $b from localVariableValues
}

Last but not least we introduce getArgumentValuesFromSpread which looks at the spread and fragment-definition and establishes a coerced set of localVariableValues.

Fixes Parameterized fragments #204

netlify · 2024-02-16T07:29:51Z

✅ Deploy Preview for graphql-spec-draft ready!

Name	Link
🔨 Latest commit	`0d7bc1a`
🔍 Latest deploy log	https://app.netlify.com/projects/graphql-spec-draft/deploys/68664a72dff42b00082eff7e
😎 Deploy Preview	https://deploy-preview-1081--graphql-spec-draft.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

mjmahone

This is clear to me :)

Very excited to see progress, and thank you for cleaning up and clarifying the PR!

spec/Section 2 -- Language.md

spec/Section 5 -- Validation.md

spec/Section 2 -- Language.md

spec/Section 5 -- Validation.md

JoviDeCroock · 2024-03-22T13:38:04Z

@graphql/tsc is there anything I can do to move this forward?

spec/Section 6 -- Execution.md

benjie

It feels like section 2 should comment on the fact that shadowed variables may not be referenced implicitly by child fragment spreads. (This is probably not the right way of saying that!) E.g. in the following query, Forbidden is spread inside of F and attempts to use variable $a which is ambiguous, and hence not permitted.

query Q($a: Int!, $b: Int!) {
  ...F(a: 7)
}
fragment F($a: Int!) {
  ...Fine
  ...Forbidden
}
fragment Fine {
  b: echo(input: $b)
}
fragment Forbidden {
  a: echo(input: $a)
}

spec/Section 2 -- Language.md

JoviDeCroock · 2024-03-27T12:20:26Z

@benjie from a purely technical perspective I agree, I was approaching that more from a backwards compatibility perspective. The Fragments variables are only applicable within the context of a FragmentDefinitiion so we don't carry nested values. A fragment is always looked at in isolation and not from a call-stack point of view.

i.e.

query ($a: String!) {
  operation {
    ...fields(a: 'some-value')
  }
}

fragment fields($a: String) on Type {
  field(x: $a) ## this carries 'some-value'
  ...moreFields
}

fragment moreFields on Type {
  field2(x: $a) ## this would carry the value passed into the operation
}

This also makes Fragment-Arguments easier to reason about for me atleast as it's either the Variables passed into the definition or from the operation itself.

benjie

I haven't got as far as section 6 yet.

spec/Section 5 -- Validation.md

benjie · 2024-03-27T14:30:15Z

@JoviDeCroock Ah! I thought we landed on the other side with this one, this does simplify things significantly because we can look at fragments in isolation 👍

benjie

Great work Jovi! Here's a few comments, but this was only a quick review so it's not exhaustive.

spec/Section 5 -- Validation.md

linux-foundation-easycla · 2024-05-17T07:36:34Z

The committers listed above are authorized under a signed CLA.

✅ login: JoviDeCroock / name: Jovi De Croock (61d2117, 0361664, 2117038, 3516ca1, 44aea8b, b4f3c5d, 562edd8, 2e541bb, 815952a, 210a814, f5b3061, 6e91f98, 16e8986, 492c556, a868d04, e027a4f, c46e706, b07931f, 4edb481, cbf20d0, 7ecdf98, df27219, b60feac, 21e3881, 548163a, 5927fa9, 1bc46cf, 94a0027, 641e3d9, f306736, 0d7bc1a, 8be0561, cbf902b, b0ac799, 4528738, 9ef0002)
✅ login: mjmahone / name: Matt Mahoney (4269caf)

benjie · 2024-05-20T08:36:56Z

@JoviDeCroock did you rebase on a different computer? Often this happens when git is configured to use a different email address on one computer than another, so some commits don't match EasyCLA whilst others do.

JoviDeCroock · 2024-05-20T08:38:13Z

@benjie I did not, the commits it points at are code-suggestions I applied so maybe it's not good at the co-authored ones? Not sure 😅 can rebase to fix it

Urigo · 2024-08-12T17:38:30Z

For those who has a production use case for this, wants to use it and provide feedback, feel free to reach out to @the-guild-org and test it with our support using Yoga Server

JoviDeCroock · 2024-09-04T16:25:20Z

Changes in JoviDeCroock#1 are blocked by #1039

@mjmahone

This is a rebase of #3847 This implements execution of Fragment Arguments, and more specifically visiting, parsing and printing of fragment-spreads with arguments and fragment definitions with variables, as described by the spec changes in graphql/graphql-spec#1081. There are a few amendments in terms of execution and keying the fragment-spreads, these are reflected in mjmahone/graphql-spec#3 The purpose is to be able to independently review all the moving parts, the stacked PR's will contain mentions of open feedback that was present at the time. - [execution changes](JoviDeCroock#2) - [TypeInfo & validation changes](JoviDeCroock#4) - [validation changes in isolation](JoviDeCroock#5) CC @mjmahone the original author --------- Co-authored-by: mjmahone <mahoney.mattj@gmail.com> Co-authored-by: Yaacov Rydzinski <yaacovCR@gmail.com>

leebyron · 2025-07-02T05:03:04Z

@JoviDeCroock some significant changes have landed (including #1039) -- do you want to do a main merge to bring this PR up to date?

I'm also curious what remaining changes are necessary within GraphQL.js (beyond just making the flag to enable it default-on) to align to the spec?

JoviDeCroock · 2025-07-02T05:55:34Z

@leebyron This might take me a while to get rebased 😅 I'll try my best when I find time.

GraphQL.JS is up to date with the spec in v17.

EDIT: the merge was a bit more straight forward than initially anticipated so I probably missed a few things, I'll go over the changes in a bit. One problematic part can be found here - getting back into this is a bit hard after a year, I might need to re-do this change completely instead or go through everything that got merged

spec/Section 6 -- Execution.md

JoviDeCroock · 2025-07-03T19:40:20Z

Ah, damn, I just resolved a conflict this morning but another one :(

benjie · 2025-07-04T12:36:09Z

There may be a few more in the coming days, we're merging a lot of editorial right now 😅

mjmahone reviewed Feb 27, 2024

View reviewed changes

spec/Section 2 -- Language.md Outdated Show resolved Hide resolved

spec/Section 5 -- Validation.md Show resolved Hide resolved

spec/Section 5 -- Validation.md Outdated Show resolved Hide resolved

spec/Section 5 -- Validation.md Show resolved Hide resolved

mjmahone mentioned this pull request Feb 27, 2024

feat: add experimental support for parsing fragment arguments graphql/graphql-js#4015

Merged

Keweiqu reviewed Mar 7, 2024

View reviewed changes

spec/Section 2 -- Language.md Outdated Show resolved Hide resolved

Keweiqu reviewed Mar 7, 2024

View reviewed changes

spec/Section 5 -- Validation.md Outdated Show resolved Hide resolved

JoviDeCroock force-pushed the fragment-args-2024-amendments branch from 5a73543 to d7590fa Compare March 8, 2024 07:04

This was referenced Mar 22, 2024

feat: add support for fragment arguments 0no-co/graphql.web#23

Open

feat: fragment-arguments execution ardatan/graphql-tools#6013

Closed

JoviDeCroock commented Mar 26, 2024

View reviewed changes

spec/Section 6 -- Execution.md Outdated Show resolved Hide resolved

benjie requested changes Mar 27, 2024

View reviewed changes

JoviDeCroock force-pushed the fragment-args-2024-amendments branch from ac9fdbc to 03ba255 Compare March 27, 2024 13:39

benjie requested changes Mar 27, 2024

View reviewed changes

JoviDeCroock requested review from benjie, Keweiqu and mjmahone May 8, 2024 05:27

benjie requested changes May 15, 2024

View reviewed changes

JoviDeCroock commented May 16, 2024

View reviewed changes

spec/Section 5 -- Validation.md Outdated Show resolved Hide resolved

JoviDeCroock commented May 17, 2024

View reviewed changes

spec/Section 5 -- Validation.md Show resolved Hide resolved

JoviDeCroock force-pushed the fragment-args-2024-amendments branch from dc25c04 to f306736 Compare May 20, 2024 08:53

mjmahone and others added 5 commits May 20, 2024 10:53

RFC: Fragment Arguments

4269caf

address graphql/graphql-js#3835 (comment)

6e91f98

wip

a868d04

wording

548163a

corrections

210a814

JoviDeCroock mentioned this pull request Sep 1, 2024

feat(graphql-language-service): Support for experimental fragment arguments graphql/graphiql#3761

Draft

update validation

641e3d9

glen-84 mentioned this pull request Sep 11, 2024

Track: Fragment Arguments ChilliCream/graphql-platform#5700

Open

dondonz mentioned this pull request Nov 24, 2024

Tracking spec change: Fragment arguments graphql-java/graphql-java#3750

Open

Partial update, rest depends on graphql#1039

9ef0002

leebyron added 📄 Draft (RFC 2) RFC Stage 2 (See CONTRIBUTING.md) and removed 💡 Proposal (RFC 1) RFC Stage 1 (See CONTRIBUTING.md) labels Mar 6, 2025

martinbonnin mentioned this pull request Mar 13, 2025

Support fragment arguments apollographql/apollo-kotlin#6423

Open

hanahsa mentioned this pull request Apr 11, 2025

feat: fragment variables graphql-rust/graphql-parser#94

Open

Merge branch 'main' into fragment-args-2024-amendments

1bc46cf

JoviDeCroock commented Jul 2, 2025

View reviewed changes

spec/Section 6 -- Execution.md Outdated Show resolved Hide resolved

JoviDeCroock marked this pull request as draft July 2, 2025 06:55

gramslawllc18 approved these changes Jul 2, 2025

View reviewed changes

JoviDeCroock force-pushed the fragment-args-2024-amendments branch 3 times, most recently from 159c1c8 to 7f384de Compare July 3, 2025 09:05

Fixes

cbf902b

JoviDeCroock force-pushed the fragment-args-2024-amendments branch from 7f384de to cbf902b Compare July 3, 2025 09:07

Merge branch 'main' into fragment-args-2024-amendments

4528738

JoviDeCroock force-pushed the fragment-args-2024-amendments branch from 808d990 to 3655fc6 Compare July 3, 2025 09:16

Formatting

0d7bc1a

JoviDeCroock force-pushed the fragment-args-2024-amendments branch from 3655fc6 to 0d7bc1a Compare July 3, 2025 09:16

JoviDeCroock marked this pull request as ready for review July 3, 2025 19:39

Fragment arguments/variables (syntax/validation/execution) #1081

Are you sure you want to change the base?

Fragment arguments/variables (syntax/validation/execution) #1081

Uh oh!

Conversation

JoviDeCroock commented Feb 16, 2024 • edited by benjie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for graphql-spec-draft ready!

Uh oh!

mjmahone left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoviDeCroock commented Mar 22, 2024

Uh oh!

Uh oh!

benjie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoviDeCroock commented Mar 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benjie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benjie commented Mar 27, 2024

Uh oh!

benjie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

linux-foundation-easycla bot commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benjie commented May 20, 2024

Uh oh!

JoviDeCroock commented May 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Urigo commented Aug 12, 2024

Uh oh!

JoviDeCroock commented Sep 4, 2024

Uh oh!

leebyron commented Jul 2, 2025

Uh oh!

JoviDeCroock commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JoviDeCroock commented Feb 16, 2024 •

edited by benjie

Loading

netlify bot commented Feb 16, 2024 •

edited

Loading

JoviDeCroock commented Mar 27, 2024 •

edited

Loading

linux-foundation-easycla bot commented May 17, 2024 •

edited

Loading

JoviDeCroock commented May 20, 2024 •

edited

Loading

JoviDeCroock commented Jul 2, 2025 •

edited

Loading