Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeInfo & Validation for fragment-arguments #4

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

JoviDeCroock
Copy link
Owner

@JoviDeCroock JoviDeCroock commented Feb 2, 2024

This implements validation + type-info for fragment arguments

Copy link

github-actions bot commented Feb 2, 2024

Hi @JoviDeCroock, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@JoviDeCroock JoviDeCroock force-pushed the fragment-args-typeinfo-2024 branch from 046432f to 2f8c76d Compare February 2, 2024 10:32
@JoviDeCroock JoviDeCroock force-pushed the fragment-args-typeinfo-2024 branch 2 times, most recently from a97a00c to 820ae11 Compare February 4, 2024 12:51
@JoviDeCroock JoviDeCroock force-pushed the fragment-args-syntax-2024 branch 3 times, most recently from 839284d to 3918bb5 Compare August 20, 2024 14:44
@JoviDeCroock JoviDeCroock force-pushed the fragment-args-typeinfo-2024 branch 5 times, most recently from 9de28dc to b422131 Compare August 20, 2024 15:40
@JoviDeCroock JoviDeCroock force-pushed the fragment-args-typeinfo-2024 branch 2 times, most recently from 8754c32 to 2fb5f1d Compare August 29, 2024 18:00
JoviDeCroock and others added 2 commits August 29, 2024 20:01
* Fragment args validation

* add experimental substitution

* validation suggestions (#12)

* validation suggestions

* remove OperationSignature from ValidationContext

only used in one rule, does not need to be on context

remove dependency on getVariableSignature

move FRAGMENT_ARGUMENT below ARGUMENT

fragment arguments do not have location defaults

only variable defaults => so getDefaultValue(), which returns location defaults for use with the allowedVariableUsage helper, should never be called by getVariableUsages with respect to fragment arguments

fragment arguments therefore need not add anything to the default value stack

reduce diff from main

these diffs crept in before Kind.FRAGMENT_ARGUMENT was separated out

---------

Co-authored-by: Yaacov Rydzinski <[email protected]>
@JoviDeCroock JoviDeCroock changed the title Add typeinfo functionality as a run-up to supporting the new validation rules TypeInfo & Validation for fragment-arguments Aug 30, 2024
@JoviDeCroock JoviDeCroock merged commit a5627c0 into fragment-args-syntax-2024 Sep 4, 2024
28 of 29 checks passed
JoviDeCroock added a commit to graphql/graphql-js that referenced this pull request Sep 6, 2024
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 <[email protected]>
Co-authored-by: Yaacov Rydzinski <[email protected]>
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.

3 participants