Skip to content

Enforces input coercion rules.#553

Merged
leebyron merged 7 commits into
masterfrom
strict-inputs
Nov 3, 2016
Merged

Enforces input coercion rules.#553
leebyron merged 7 commits into
masterfrom
strict-inputs

Conversation

@leebyron

@leebyron leebyron commented Nov 1, 2016

Copy link
Copy Markdown
Contributor

Before this diff, bad input to arguments and variables was often ignored and replaced with null rather than rejected. Now that null has a semantic meaning, and thanks to some recent changes to the spec (graphql/graphql-spec#221) - changes are necessary in order to enforce the stricter coercion rules.

This diff does the following:

  • Implements the CoerceArgumentValues as described in the spec.
  • Implements the CoerceVariablesValues as described in the spec.
  • Alters valueFromAST and coerceValue (dual functions) to strictly enforce coercion, returning undefined implicitly when they fail to do so. It also fixes issues where undefined returns were being ignored as items in a list or fields in an input object.

Before this diff, bad input to arguments and variables was often ignored and replaced with `null` rather than rejected. Now that `null` has a semantic meaning, and thanks to some recent changes to the spec (graphql/graphql-spec#221) - changes are necessary in order to enforce the stricter coercion rules.

This diff does the following:

* Implements the CoerceArgumentValues as described in the spec.
* Implements the CoerceVariablesValues as described in the spec.
* Alters valueFromAST and coerceValue (dual functions) to strictly enforce coercion, returning `undefined` implicitly when they fail to do so. It also fixes issues where undefined returns were being ignored as items in a list or fields in an input object.
@leebyron

leebyron commented Nov 1, 2016

Copy link
Copy Markdown
Contributor Author

Some of this ends up being follow-up to @langpavel's #544 as edges cases with support for null interrelates to the strict coercion rules.

@langpavel

Copy link
Copy Markdown
Contributor

Well done!

@thebigredgeek

Copy link
Copy Markdown

@leebyron it appears that errors thrown from parseValue in scalar resolvers are now completely swallowed, with their messages being appended to the invalid input message. This is a considerable breaking change, as many people are using scalars for form validation (myself included) and this makes it impossible to propagate back custom error text for presentation on the client. Is there any way that the error could retain a reference to the originalError as is the case with most other errors?

@leebyron

Copy link
Copy Markdown
Contributor Author

Certainly retaining a reference to originalError is intended. I'd support a PR which fixed this issue

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