Argument and variables nullability #64
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Specification does not explicitly forbid using defaults with Non-null variables. The discussion [1] states that it is legal. Reference JS implementation supports defaults for Non-null variables. 1. graphql/graphql-spec#570
Specification [1] states that Nullable variable can be used for NonNullable argument if it has Non-null default. See more in [2] issue. 1. http://spec.graphql.org/October2021/#sec-All-Variable-Usages-are-Allowed 2. graphql/graphql-js#3715
Code uses NonNull wrapper to support case when Nullable variable has a Non-null default value and is compatible with NonNullable argument. But if it really incompatible, the user should receive an error with original type in message.
Compare library with JS reference implementation in various nullability cases. This patch uses JS script to build various schemas and queries and execute them to generate luatest script that verifies that this library has the same behavior. To generate script, run ```bash cd ./tests/integration/codegen/fuzzing_nullability npm install node ./generate.js > ../../fuzzing_nullability_test.lua ``` Prior to this patch, several nullability behavior issues were fixed. There is some case that isn't yet fully covered by our library, refer to [1] for updates. Follow [2] for more test cases. Based on @no1seman PR [3]. 1. #62 2. #63 3. #59
This was referenced Aug 26, 2022
DifferentialOrange
deleted the
DifferentialOrange/variables_nullability_v2
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reworked and improved version of #59
Compare library with JS reference implementation in various nullability cases. This PR uses JS script to build various schemas and queries and execute them to generate luatest script that verifies that this library has the same behavior.
To generate script, run
This PR fixed several nullability behavior issues. There is some case that isn't yet fully covered by our library, refer to #62 for updates.
Follow #63 for more test cases.