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

Validate literals in a single rule with finer precision #1144

Merged
merged 1 commit into from
Dec 14, 2017

Conversation

leebyron
Copy link
Contributor

This generalizes the "arguments of correct type" and "default values of correct type" to a single rule "values of correct type" which has been re-written to rely on a traversal rather than the utility function isValidLiteralValue. To reduce breaking scope, this does not remove that utility even though it's no longer used directly within the library. Since the default values rule included another validation rule that rule was renamed to a more apt "variable default value allowed".

This also includes the original errors from custom scalars in the validation error output, solving the remainder of #821.

Closes #821

@leebyron
Copy link
Contributor Author

I plan to open a PR making non-normative edits to the spec which better align to this refactoring as well

This generalizes the "arguments of correct type" and "default values of correct type" to a single rule "values of correct type" which has been re-written to rely on a traversal rather than the utility function `isValidLiteralValue`. To reduce breaking scope, this does not remove that utility even though it's no longer used directly within the library. Since the default values rule included another validation rule that rule was renamed to a more apt "variable default value allowed".

This also includes the original errors from custom scalars in the validation error output, solving the remainder of #821.

Closes #821
@leebyron leebyron merged commit ada56fe into master Dec 14, 2017
@leebyron leebyron deleted the literal-validation branch December 14, 2017 21:56
leebyron added a commit to graphql/graphql-spec that referenced this pull request Dec 15, 2017
This PR is an editorial change to validation rules which does not affect which documents are valid or invalid, however seeks to generalize the language for validating input value literals and in doing so both de-duplicate the rules (previously duplicated between argument validation and default value validation) and ensure the validation of value literals applies to any new location values may be found in the future (such as the SDL).

This change to the spec mirrors a similar change to the reference implementation graphql/graphql-js#1144
leebyron added a commit to graphql/graphql-spec that referenced this pull request Dec 18, 2017
This PR is an editorial change to validation rules which does not affect which documents are valid or invalid, however seeks to generalize the language for validating input value literals and in doing so both de-duplicate the rules (previously duplicated between argument validation and default value validation) and ensure the validation of value literals applies to any new location values may be found in the future (such as the SDL).

This change to the spec mirrors a similar change to the reference implementation graphql/graphql-js#1144
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants