Skip to content

Commit

Permalink
Updated based on feedback.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
leebyron committed Mar 30, 2018
1 parent ef26891 commit 3dc6d1b
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 88 deletions.
96 changes: 44 additions & 52 deletions spec/Section 5 -- Validation.md
Original file line number Diff line number Diff line change
Expand Up @@ -710,25 +710,26 @@ and invalid.
* {arguments} must be the set containing only {argument}.


#### Required Non-Null Arguments
#### Required Arguments

* For each Field or Directive in the document.
* Let {arguments} be the arguments provided by the Field or Directive.
* Let {argumentDefinitions} be the set of argument definitions of that Field or Directive.
* For each {definition} in {argumentDefinitions}:
* Let {type} be the expected type of {definition}.
* If {type} is Non-Null:
* Let {argumentName} be the name of {definition}.
* For each {argumentDefinition} in {argumentDefinitions}:
* Let {type} be the expected type of {argumentDefinition}.
* Let {defaultValue} be the default value of {argumentDefinition}.
* If {type} is Non-Null and {defaultValue} does not exist:
* Let {argumentName} be the name of {argumentDefinition}.
* Let {argument} be the argument in {arguments} named {argumentName}
* {argument} must exist.
* Let {value} be the value of {argument}.
* {value} must not be the {null} literal.

**Explanatory Text**

Arguments can be required. If the argument type is non-null the argument is
required and furthermore the explicit value {null} may not be provided.
Otherwise, the argument is optional.
Arguments can be required. If the argument type is non-null and does not have a
default value, the argument is required and furthermore the explicit value
{null} may not be provided. Otherwise, the argument is optional.

For example the following are valid:

Expand All @@ -738,7 +739,7 @@ fragment goodBooleanArg on Arguments {
}

fragment goodNonNullArg on Arguments {
nonNullBooleanArgField(nonNullBooleanArg: true)
requiredBooleanArgField(requiredBooleanArg: true)
}
```

Expand All @@ -752,19 +753,19 @@ fragment goodBooleanArgDefault on Arguments {
}
```

but this is not valid on a non-null argument.
but this is not valid on a required argument.

```graphql counter-example
fragment missingRequiredArg on Arguments {
nonNullBooleanArgField
requiredBooleanArgField
}
```

Providing the explicit value {null} is also not valid.

```graphql counter-example
fragment missingRequiredArg on Arguments {
notNullBooleanArgField(nonNullBooleanArg: null)
requiredBooleanArgField(requiredBooleanArg: null)
}
```

Expand Down Expand Up @@ -1358,6 +1359,31 @@ For example the following query will not pass validation.
```


### Input Object Required Fields

**Formal Specification**

* For each Input Object in the document.
* Let {fields} be the fields provided by that Input Object.
* Let {fieldDefinitions} be the set of input field definitions of that Input Object.
* For each {fieldDefinition} in {fieldDefinitions}:
* Let {type} be the expected type of {fieldDefinition}.
* Let {defaultValue} be the default value of {fieldDefinition}.
* If {type} is Non-Null and {defaultValue} does not exist:
* Let {fieldName} be the name of {fieldDefinition}.
* Let {field} be the input field in {fields} named {fieldName}
* {field} must exist.
* Let {value} be the value of {field}.
* {value} must not be the {null} literal.

**Explanatory Text**

Input object fields can be required. If the input object field type is non-null
and does not have a default value, the input object field is required and
furthermore the explicit value {null} may not be provided. Otherwise, the input
object field is optional.


## Directives


Expand Down Expand Up @@ -1494,44 +1520,6 @@ fragment HouseTrainedFragment {
```


### Variable Default Value Is Allowed

**Formal Specification**

* For every Variable Definition {variableDefinition} in a document
* Let {variableType} be the type of {variableDefinition}
* Let {defaultValue} be the default value of {variableDefinition}
* If {variableType} is Non-null:
* {defaultValue} must be undefined.

**Explanatory Text**

Variables defined by operations are allowed to define default values
if the type of that variable is not non-null.

For example the following query will pass validation.

```graphql example
query houseTrainedQuery($atOtherHomes: Boolean = true) {
dog {
isHousetrained(atOtherHomes: $atOtherHomes)
}
}
```

However if the variable is defined as non-null, default values
are unreachable. Therefore queries such as the following fail
validation

```graphql counter-example
query houseTrainedQuery($atOtherHomes: Boolean! = true) {
dog {
isHousetrained(atOtherHomes: $atOtherHomes)
}
}
```


### Variables Are Input Types

**Formal Specification**
Expand Down Expand Up @@ -1906,8 +1894,12 @@ query booleanArgQuery($booleanArg: Boolean) {
}
```

A notable exception is when a default value are provided. They are, in effect,
treated as non-nulls as long as the default value is not {null}.
A notable exception is when a default value is provided. Variables which include
a default value may be provided to a non-null argument as long as the default
value is not {null}.

Note: The value {null} could still be provided to a variable at runtime, and
a non-null argument must produce a field error if provided such a variable.

```graphql example
query booleanArgQueryWithDefault($booleanArg: Boolean = true) {
Expand Down
74 changes: 38 additions & 36 deletions spec/Section 6 -- Execution.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,21 +88,22 @@ CoerceVariableValues(schema, operation, variableValues):
name {variableName}.
* Let {value} be the value provided in {variableValues} for the
name {variableName}.
* If {hasValue} is not {true} or if {value} is {null}:
* If {variableType} is a Non-Nullable type, throw a query error.
* Otherwise if {defaultValue} exists (including {null}):
* Add an entry to {coercedValues} named {variableName} with the
value {defaultValue}.
* Otherwise if {hasValue} is {true} and {value} is {null}:
* If {hasValue} is not {true} and {defaultValue} exists (including {null}):
* Add an entry to {coercedValues} named {variableName} with the
value {defaultValue}.
* Otherwise if {variableType} is a Non-Nullable type, and either {hasValue}
is not {true} or {value} is {null}, throw a query error.
* Otherwise if {hasValue} is true:
* If {value} is {null}:
* Add an entry to {coercedValues} named {variableName} with the
value {null}.
* Otherwise, {value} must not be {null}:
* If {value} cannot be coerced according to the input coercion
rules of {variableType}, throw a query error.
* Let {coercedValue} be the result of coercing {value} according to the
input coercion rules of {variableType}.
* Add an entry to {coercedValues} named {variableName} with the
value {coercedValue}.
* Otherwise:
* If {value} cannot be coerced according to the input coercion
rules of {variableType}, throw a query error.
* Let {coercedValue} be the result of coercing {value} according to the
input coercion rules of {variableType}.
* Add an entry to {coercedValues} named {variableName} with the
value {coercedValue}.
* Return {coercedValues}.

Note: This algorithm is very similar to {CoerceArgumentValues()}.
Expand Down Expand Up @@ -552,34 +553,35 @@ CoerceArgumentValues(objectType, field, variableValues):
* Let {defaultValue} be the default value for {argumentDefinition}.
* Let {hasValue} be {true} if {argumentValues} provides a value for the
name {argumentName}.
* Let {value} be the value provided in {argumentValues} for the
* Let {argumentValue} be the value provided in {argumentValues} for the
name {argumentName}.
* If {hasValue} is not {true} or if {value} is {null}:
* If {argumentType} is a Non-Nullable type, throw a field error.
* Otherwise, if {defaultValue} exists (including {null}):
* Add an entry to {coercedValues} named {argumentName} with the
value {defaultValue}.
* Otherwise, if {hasValue} is {true} and {value} is {null}:
* If {argumentValue} is a {Variable}:
* Let {variableName} be the name of {argumentValue}.
* Let {hasValue} be {true} if {variableValues} provides a value for the
name {variableName}.
* Let {value} be the value provided in {variableValues} for the
name {variableName}.
* Otherwise:
* Let {value} be {argumentValue}.
* If {hasValue} is not {true} and {defaultValue} exists (including {null}):
* Add an entry to {coercedValues} named {argumentName} with the
value {defaultValue}.
* Otherwise if {argumentType} is a Non-Nullable type, and either {hasValue}
is not {true} or {value} is {null}, throw a field error.
* Otherwise if {hasValue} is true:
* If {value} is {null}:
* Add an entry to {coercedValues} named {argumentName} with the
value {null}.
* Otherwise, if {value} is a {Variable}:
* Let {variableName} be the name of {value}.
* If {variableValues} provides a value for the name {variableName}:
* Let {variableValue} be the value provided in {variableValues} for the
name {variableName}.
* Otherwise, if {argumentValue} is a {Variable}:
* Add an entry to {coercedValues} named {argumentName} with the
value {variableValue}.
* Otherwise, if {argumentType} is a Non-Nullable type, throw a field error.
* Otherwise, if {defaultValue} exists (including {null}):
value {value}.
* Otherwise:
* If {value} cannot be coerced according to the input coercion
rules of {variableType}, throw a field error.
* Let {coercedValue} be the result of coercing {value} according to the
input coercion rules of {variableType}.
* Add an entry to {coercedValues} named {argumentName} with the
value {defaultValue}.
* Otherwise, {value} must be a {Value} and not {null}:
* If {value} cannot be coerced according to the input coercion
rules of {argType}, throw a field error.
* Let {coercedValue} be the result of coercing {value} according to the
input coercion rules of {argType}.
* Add an entry to {coercedValues} named {argumentName} with the
value {coercedValue}.
value {coercedValue}.
* Return {coercedValues}.

Note: Variable values are not coerced because they are expected to be coerced
Expand Down

0 comments on commit 3dc6d1b

Please sign in to comment.