graphql-js icon indicating copy to clipboard operation
graphql-js copied to clipboard

RFC: Default value validation & coercion

Open yaacovCR opened this issue 2 years ago • 3 comments

#3049 rebased on main.

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

  • 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

yaacovCR avatar Jan 02 '23 10:01 yaacovCR

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
Latest commit 57e32d78041e0263991b7ae2488c62fde32ae930
Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/64773bad17553e00087548f1
Deploy Preview https://deploy-preview-3814--compassionate-pike-271cb3.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] avatar Jan 02 '23 10:01 netlify[bot]

Hi @yaacovCR, 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

github-actions[bot] avatar Jan 02 '23 10:01 github-actions[bot]

A changelog entry for the first 2 changes in the PR stack might read as follows:

** Implements spec changes around the introduction of the Schema Coordinate nomenclature.

  • Introduces new parser method: parseSchemaCoordinate() which can parse a string/source schema coordinate into an AST .
  • Introduces new methods resolveSchemaCoordinate() and resolveASTSchemeCoordinate() which resolve unparsed and parsed schema coordinates in a particular schema context, returning a ResolvedSchemaElement.
  • Introduces the new discriminated union ResolvedSchemaElement to represent the result of a schema coordinate resolved in the context of a particular schema.
  • Improves error messages across the code base using the coordinate nomenclature.

For the next 8 changes, an overall changelog entry might read as follows:

** Implements spec changes around default value validation. ** Fixes introspection not always returning the original SDL supplied default value. ** Fixes custom scalars not required to replacing variables correctly.

BREAKING CHANGES:

  • A default values is now supplied programmatically as an "External input value" rather than "Internal input value"; the value is now coerced by coerceInputValue() using the appropriate coercion rules. This is a significant BREAKING CHANGE, and will likely require schema authors to check and convert all programmatically supplied default values.

    • To help with this transition, if we can surmise that an default value was supplied as an "Internal input value," a helpful error message will be reported. This error is reported if (a) validation of the default value fails; (b) we can "uncoerce" the default value by treating it like an output value and attempting to serialize it; (c) validation of the "uncoerced" default value succeeds.
    • Introduces a memoized method coerceDefaultValue() that coerces default values according to the appropriate rules.
    • Schema element defaultValue attributes have been replaced with objects of new type GraphQLDefaultValueUsage that include either the original literal value or the uncoerced "External input value."
  • Replaces astFromValue() entirely with the new method valueToLiteral() for use in introspection and schema printing.

    • astFromValue() performed unsafe "uncoercion" from an "Internal input value" to a "GraphQL Literal AST", where valueToLiteral() performs a well defined transform from "External input value" to "GraphQL Literal AST".
  • Replaces valueFromAST() with new method coerceInputLiteral().

    • coerceInputLiteral() now utilizes new replaceVariables() to replaces any variables found within an AST Value literal with their corresponding literals so as to ensure that values generates from ASTs are always constant.
    • The value provided to scalar parseLiteral nodes is now ConstValueNode; the second variables argument has been removed.
  • coerceInputValue() no longer performs error processing, instead simply returning early with value of undefined.

    • New validateInputValue() and validateInputLiteral() methods extracted from coerceInputValue and ValuesOfCorrectTypeRule, respectively, perform unified error reporting.
    • coerceVariableValues() still appropriately reports errors: an undefinedresults from coerceInputValue() will trigger a call to validateInputValue() that will generate the appropriate error.
  • The return value of coerceVariableValues() has changed to an object of new type VariableValues that preserves the original sources (including the default values), a change that enables the behavior of replaceVariables().

yaacovCR avatar Feb 21 '23 11:02 yaacovCR

Help androad

Rodipaun avatar Oct 15 '23 13:10 Rodipaun