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 coerceInputLiteral() #3092

Closed
wants to merge 5 commits into from
Closed

Conversation

leebyron
Copy link
Contributor

@leebyron leebyron commented May 15, 2021

Depends on #3145

Removes valueFromAST() and adds coerceInputLiteral() as an additional export from coerceInputValue.

The implementation is almost exactly the same as valueFromAST() with a slightly more strict type signature and refactored tests to improve coverage (the file unit test has 100% coverage)

While this does not change any behavior, it could be breaking if you rely directly on the valueFromAST() method. Use coerceInputLiteral() as a direct replacement.

@leebyron leebyron force-pushed the coerce-input-literal branch from ce44449 to 5e2b6d5 Compare May 15, 2021 04:51
@leebyron leebyron force-pushed the coerce-input-literal branch from 5e2b6d5 to e4243d8 Compare May 15, 2021 06:50
@leebyron leebyron force-pushed the coerce-input-literal branch from e4243d8 to 7dd6206 Compare May 15, 2021 21:31
@leebyron leebyron force-pushed the coerce-input-literal branch from 7dd6206 to 2f04eed Compare June 1, 2021 18:56
@leebyron leebyron changed the base branch from input-value-config to definition-coordinates June 1, 2021 18:57
@leebyron leebyron added PR: internal 🏠 PR: breaking change 💥 implementation requires increase of "major" version number and removed PR: internal 🏠 labels Jun 1, 2021
@leebyron leebyron force-pushed the definition-coordinates branch from 9d5277a to 4e4d16e Compare June 1, 2021 21:42
@leebyron leebyron force-pushed the coerce-input-literal branch 2 times, most recently from 15223bc to a79c0e5 Compare June 1, 2021 22:06
leebyron and others added 4 commits June 3, 2021 09:51
The lexer needed some cleanup, I found myself doing this as part of a Unicode RFC, but factoring all that out to make the Unicode RFC PR easier to follow.

* Always use hexadecimal form for code values.
* Remove use of `isNaN` for checking source over-reads.
* Defines `isSourceCharacter`
* Add more documentation and comments, also replaces regex with lexical grammar
* Simplifies error messages
* Adds additional tests
Co-authored-by: Ivan Goncharov <[email protected]>
Implements graphql/graphql-spec#794

Adds:

* DOT punctuator in lexer
* Improvements to lexer errors around misuse of `.`
* Minor improvement to parser core which simplified this addition
* `SchemaCoordinate` node and `isSchemaCoodinate()` predicate
* Support in `print()` and `visit()`
* Added function `parseSchemaCoordinate()` since it is a parser entry point.
* Added function `resolveSchemaCoordinate()` and `resolveASTSchemaCoordinate()` which implement the semantics (name mirrored from `buildASTSchema`) as well as the return type `ResolvedSchemaElement`
* Defines a `GraphQLSchemaElement` base class which defines a `.coordinate` property and `toString`/`toJSON` methods.
* Adds base class to types, fields, arguments, input fields, enum values, and directives.
* Uses this in validation error printing string templates.
@leebyron leebyron force-pushed the definition-coordinates branch from 4e4d16e to a4c7cea Compare June 3, 2021 17:02
Removes `valueFromAST()` and adds `coerceInputLiteral()` as an additional export from `coerceInputValue`.

The implementation is almost exactly the same as `valueFromAST()` with a slightly more strict type signature and refactored tests to improve coverage (the file unit test has 100% coverage)

While this does not change any behavior, it could be breaking if you rely directly on the valueFromAST() method. Use `coerceInputLiteral()` as a direct replacement.
@leebyron leebyron force-pushed the coerce-input-literal branch from a79c0e5 to 8abb052 Compare June 3, 2021 17:08
@leebyron leebyron force-pushed the definition-coordinates branch from a4c7cea to b8ee9a2 Compare June 3, 2021 17:53
@yaacovCR
Copy link
Contributor

Merged as #3809

@yaacovCR yaacovCR closed this Sep 23, 2024
yaacovCR added a commit that referenced this pull request Oct 27, 2024
[#3049 rebased on
main](#3049).

This is the last rebased PR from the original PR stack concluding with
#3049.

* Rebased: #3809 [Original: #3092]
* Rebased: #3810 [Original: #3074]
* Rebased: #3811 [Original: #3077]
* Rebased: #3812 [Original: #3065]
* Rebased: #3813 [Original: #3086]
* Rebased: #3814 (this PR) [Original: #3049]

Update: #3044 and #3145 have been separated from this stack.

Changes from original PR:
1. `astFromValue()` is deprecated instead of being removed.

@leebyron comments from #3049, the original PR:
> Implements
[graphql/graphql-spec#793](graphql/graphql-spec#793)
> 
> * BREAKING: 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()`)
> To support this change in value type, this PR adds two related
behavioral changes:
>   
> * Adds coercion of default values from external to internal at runtime
(within `coerceInputValue()`)
> * Removes `astFromValue()`, replacing it with `valueToLiteral()` for
use in introspection and schema printing. `astFromValue()` performed
unsafe "uncoercion" to convert an "Internal input value" directly to a
"GraphQL Literal AST", where `valueToLiteral()` performs a well defined
transform from "External input value" to "GraphQL Literal AST".
> * Adds validation of default values during schema validation.
> Since assumed-coerced "internal" values may not pass "external"
validation (for example, Enum values), an additional validation error
has been included which attempts to detect this case and report a
strategy for resolution.
> 
> Here's a broad overview of the intended end state:
> 
> ![GraphQL Value
Flow](https://user-images.githubusercontent.com/50130/118379946-51ac5300-b593-11eb-839f-c483ecfbc875.png)

---------

Co-authored-by: Lee Byron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: breaking change 💥 implementation requires increase of "major" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants