-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
RFC: Fix ambiguity with null variable values and default values #418
Commits on Mar 6, 2018
-
RFC: Fix ambiguity with null variable values and default values
> This is a **behavioral change** which changes how explicit `null` values interact with variable and argument default values. This also changes a validation rule which makes the rule more strict. There is currently ambiguity and inconsistency in how `null` values are coerced and resolved as part of variable values, default values, and argument values. This inconsistency and ambiguity can allow for `null` values to appear at non-null arguments, which might result in unforseen null-pointer-errors. This appears in three distinct but related issues: **Validation: All Variable Usages are Allowed** The explicit value `null` may be used as a default value for a variable with a nullable type, however this rule asks to treat a variable's type as non-null if it has a default value. Instead this rule should specifically only treat the variable's type as non-null if the default value is not `null`. Additionally, the `AreTypesCompatible` algorithm is underspecificied, which could lead to further misinterpretation of this validation rule. **Coercing Variable Values** `CoerceVariableValues()` allows the explicit `null` value to be used instead of a default value. This can result in a null value flowing to a non-null argument due to the validation rule mentioned above. Instead a default value must be used even when an explicit `null` value is provided. This is also more consistent with the explanation for validation rule "Variable Default Value Is Allowed" Also, how to treat an explicit `null` value is currently underspecified. While an input object explains that a `null` value should result in an explicit `null` value at the input object field, there is no similar explaination for typical scalar input types. Instead, `CoerceVariableValues()` should explicitly handle the `null` value to make it clear a `null` is the resulting value in the `coercedValues` Map. **Coercing Argument Values** The `CoerceArgumentValues()` algorithm is intentionally similar to `CoerceVariableValues()` and suffers from the same inconsistency. Explicit `null` values should not take precedence over default values, and should also be explicitly handled rather than left to underspecified input scalar coercion.
Configuration menu - View commit details
-
Copy full SHA for ef26891 - Browse repository at this point
Copy the full SHA ef26891View commit details
Commits on Mar 30, 2018
-
This updates this proposal to be a bit broader in scope however much narrower in breaking behavior changes. Mirroring the changes in graphql/graphql-js#1274, this update better defines the difference between a "required" and "non-null" argument / input field as a non-null typed argument / input-field with a default value is no longer required. As such the validation rule which prohibited queries from using non-null variables and default values has been removed. This also adds clarity to the input field validation - this rule has existed in the GraphQL.js reference implementation however was found missing within the spec. This also updates the CoerceVariableValues() and CoerceArgumentValues() algorithms to retain explicit null values overriding a default value (minimizing breaking changes), however critically adding additional protection to CoerceArgumentValues() to explicitly block null values from variables - thus allowing the older pattern of passing a nullable variable into a non-null argument while limiting the problematic case of an explicit null value at runtime.
Configuration menu - View commit details
-
Copy full SHA for 3dc6d1b - Browse repository at this point
Copy the full SHA 3dc6d1bView commit details
Commits on Apr 3, 2018
-
One step further towards the idealized "from scratch" proposal, this …
…makes it more explicitly clear that changing the effective type of a variable definition is only relevent when supporting legacy clients and suggests that new services should not use this behavior. I like that this balances a clear description of how this rule should work for existing services along with a stricter and therefore safer future path for new services.
Configuration menu - View commit details
-
Copy full SHA for 2fbeb5d - Browse repository at this point
Copy the full SHA 2fbeb5dView commit details
Commits on Apr 5, 2018
-
Editing AreTypesCompatible() to avoid trailing "Otherwise return fals…
…e" statements for easier reading. Functionality is equivalent.
Configuration menu - View commit details
-
Copy full SHA for ee6220b - Browse repository at this point
Copy the full SHA ee6220bView commit details
Commits on Apr 6, 2018
-
Update "All Variable Usages are Allowed" to remove breaking change.
Also attempts to improve clarity and formatting and adds an example case.
Configuration menu - View commit details
-
Copy full SHA for 429bf2b - Browse repository at this point
Copy the full SHA 429bf2bView commit details -
Configuration menu - View commit details
-
Copy full SHA for 3147689 - Browse repository at this point
Copy the full SHA 3147689View commit details
Commits on Apr 18, 2018
-
Configuration menu - View commit details
-
Copy full SHA for 03197f8 - Browse repository at this point
Copy the full SHA 03197f8View commit details