graphql / graphql/graphql-spec

Field error from list arg with nullable variable entry (nullable=optional clash?)

Open
#1,002 6 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
14.6k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

Consider the following _currently valid_ schema and query:

```graphql
type Query {
sum(numbers:[Int!]!): Int
}

query Q ($number: Int = 3) {
sum(numbers: [1, $number, 3])
}
```

With the following variables:

```json
{
"number": null
}
```

Now following through [6.1.2 Coercing Variable Values](https://spec.graphql.org/draft/#sec-Coercing-Variable-Values):

1. Let coercedValues be an empty unordered Map. ✅
2. Let variablesDefinition be the variables defined by operation. ✅
3. For each variableDefinition in variablesDefinition: ✅
1. Let variableName be the name of variableDefinition. ✅
2. Let variableType be the expected type of variableDefinition. ✅
3. Assert: [IsInputType](https://spec.graphql.org/draft/#IsInputType())(variableType) must be true. ✅
4. Let defaultValue be the default value for variableDefinition. ✅
5. Let hasValue be true if variableValues provides a value for the name variableName. ✅
6. Let value be the value provided in variableValues for the name variableName. ✅
7. If hasValue is not true and defaultValue exists (including null):
1. Add an entry to coercedValues named variableName with the value defaultValue.
8. Otherwise if variableType is a Non-Nullable type, and either hasValue is not true or value is null, raise a [request error](https://spec.graphql.org/draft/#request-error).
9. Otherwise if hasValue is true: ✅
1. If value is null: ✅
1. Add an entry to coercedValues named variableName with the value null. ✅

For the variable definition `$number: Int = 3`:

- `variableType` is `Int` (NOT `Int!`).
- `defaultValue` is `3`
- `hasValue` is `true` since `number` is provided in variables (even though it's null)
- `value` is `null`

Thus `coercedValues` becomes `{ number: null }`.

When it comes to executing the field, this results in [CoerceArgumentValues()](https://spec.graphql.org/draft/#CoerceArgumentValues()) raising a field error at 5.j.iii.1 (since `[1, null , 3]` cannot be coerced to `[Int!]!`).

Had the query have been defined with `($number: Int! = 3)` then this error could not have occurred; but we explicitly allow the nullable Int with default in [IsVariableUsageAllowed()](https://spec.graphql.org/draft/#IsVariableUsageAllowed()). Presumably this is so that we can allow a default value to apply in the list, except this is a `ListValue` so there cannot be a default value there.

We've discussed "optionality" versus "nullability" a few times, but I'd like to get the WG's read on this.

Reproduction:

```js
const { graphqlSync, GraphQLSchema, GraphQLList, GraphQLNonNull, GraphQLInt, GraphQLObjectType, validateSchema } = require('graphql');

const Query = new GraphQLObjectType({
name: 'Query',
fields: {
sum: {
type: GraphQLInt,
args: {
numbers: {
type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(GraphQLInt))),
}
},
resolve(_, args) {
let total = 0;
for (const n of args.numbers) {
total += n;
}
return total;
}
}
}
});

const schema = new GraphQLSchema({
query: Query
});

const errors = validateSchema(schema);
if (errors.length > 0) {
throw errors[0];
}

{
console.log('Testing query with variables and null');
const result = graphqlSync({
schema,
source: `query Q($number: Int = 3) {sum(numbers:[1,$number,3])}`,
variableValues: {
number: null
}
});
console.dir(result);
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.