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

Add status check for default value PR stack #1248

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

yaacovCR
Copy link
Contributor

should just take a minute or two, just want to re-iterate the call(s?) to action involved

agendas/2023/02-Feb/02-wg-primary.md Outdated Show resolved Hide resolved
should just take a minute or two, just want to re-iterate the call(s?) to action involved
@benjie benjie changed the title add status check for default value PR stack Add status check for default value PR stack Jan 31, 2023
@benjie benjie merged commit 356fc80 into graphql:main Jan 31, 2023
@benjie
Copy link
Member

benjie commented Jan 31, 2023

@yaacovCR How well do you feel the Spec PR matches up with the GraphQL.js implementation? IIRC (and this was over a year ago, so my memory is very fuzzy on the subject) the GraphQL.js implementation evolved and I wasn't sure whether or not the spec reflected it any more - given you recently rebased both maybe you have a better feel for it?

@yaacovCR yaacovCR deleted the default-values branch January 31, 2023 18:20
@yaacovCR
Copy link
Contributor Author

@benjie -- considering the number of changes, it is certainly tough to say, and so the more eyes on board the better!

The main breaking change within graphql-js (besides function names/signatures) is that default values when supplied programmatically should be in the same domain as variable values, i.e. they should be external, and not internal.

To quote @leebyron see graphql/graphql-js#3814 referencing the earlier PR:

Changes default values from being represented as an assumed-coerced "internal input value" to a pre-coerced "external input value" (See chart below).
This allows programmatically provided default values to be represented in the same domain as values sent to the service via variable values, and allows it to have well defined methods for both transforming into a printed GraphQL literal string for introspection / schema printing (via valueToLiteral()) or coercing into an "internal input value" for use at runtime (via coerceInputValue())

This seems to still match the spec PR change pretty well!

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