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.

Sign up for GitHub

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

Jump to bottom

[RFC] Introduce Strict and Legacy All Variable Usages Are Allowed #1059

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[RFC] Introduce Strict and Legacy All Variable Usages Are Allowed #1059

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 181 additions & 19 deletions spec/Section 5 -- Validation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1855,6 +1855,26 @@ variable.

### All Variable Usages Are Allowed

As GraphQL evolves and issues are spotted and fixed, the specification aims to
maintain compatibility with existing GraphQL documents. In rare cases, this
requires two versions of an algorithm, a Legacy Algorithm which supports clients
with both old and new GraphQL documents, and a Strict Algorithm which only
supports new GraphQL documents. It is recommended that you implement the Strict
Algorithm unless you have existing GraphQL documents that you need to support
which would become invalid under the Strict Algorithm, in which case you should
implement the Legacy Algorithm.
Comment on lines +1864 to +1865
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"In which case you must implement the Legacy Algorithm until the existing documents are migrated to satisfy the strict algorithm"


The All Variable Usages Are Allowed validation rule has become stricter since
the release of the GraphQL Specification, specifically the Strict Algorithm no
longer allows a nullable variable to be used in a non-nullable position even
when either or both of the variable definition and the input position have a
default value.

#### Strict All Variable Usages Are Allowed

Implement this only if you are not implementing
[Legacy All Variable Usages Are Allowed](#sec-Legacy-All-Variable-Usages-Are-Allowed).
Comment on lines +1875 to +1876
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure "Implement this only if" is what we want. If you implement legacy then we also want you implementing strict.

Maybe we should have validation warnings? Where if you implement both:

  • if strict validation succeeds, no error
  • if legacy validation succeeds, warn
  • if legacy validation fails, fail.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementing both would be redundant, wouldn’t it? But yeah, this shouldn’t be a strict rule, probably a “we recommend you implement this only if” instead, from an efficiency POV.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I like the warn idea.)


**Formal Specification**

- For each {operation} in {document}:
Expand All @@ -1868,6 +1888,144 @@ variable.

IsVariableUsageAllowed(variableDefinition, variableUsage):

- Let {variableType} be the expected type of {variableDefinition}.
- Let {locationType} be the expected type of the {Argument}, {ObjectField}, or
{ListValue} entry where {variableUsage} is located.
- Return {AreTypesCompatible(variableType, locationType)}.

**Explanatory Text**

Variable usages must be compatible with the arguments they are passed to.

Validation failures occur when variables are used in the context of types that
are complete mismatches, or if a nullable type in a variable is passed to a
non-null argument type.

Types must match:

```graphql counter-example
query intCannotGoIntoBoolean($intArg: Int) {
arguments {
booleanArgField(booleanArg: $intArg)
}
}
```

${intArg} typed as {Int} cannot be used as an argument to {booleanArg}, typed as
{Boolean}.

List cardinality must also be the same. For example, lists cannot be passed into
singular values.

```graphql counter-example
query booleanListCannotGoIntoBoolean($booleanListArg: [Boolean]) {
arguments {
booleanArgField(booleanArg: $booleanListArg)
}
}
```

Nullability must also be respected. A nullable variable cannot be passed to a
non-null argument.

```graphql counter-example
query booleanArgQuery($booleanArg: Boolean) {
arguments {
nonNullBooleanArgField(nonNullBooleanArg: $booleanArg)
}
}
```

For list types, the same rules around nullability apply to both outer types and
inner types. A nullable list cannot be passed to a non-null list, and a list of
nullable values cannot be passed to a list of non-null values. The following is
valid:

```graphql example
query nonNullListToList($nonNullBooleanList: [Boolean]!) {
arguments {
booleanListArgField(booleanListArg: $nonNullBooleanList)
}
}
```

However, a nullable list cannot be passed to a non-null list:

```graphql counter-example
query listToNonNullList($booleanList: [Boolean]) {
arguments {
nonNullBooleanListField(nonNullBooleanListArg: $booleanList)
}
}
```

This would fail validation because a `[T]` cannot be passed to a `[T]!`.
Similarly a `[T]` cannot be passed to a `[T!]`.

**Nullability, Optionality, and Default Values**

The value for a non-nullable variable with a default value may be omitted, or
may be set to a non-null value, but it cannot be explicitly {null}. Thus, rather
than handling a {null} with a run-time _field error_ as in Legacy All Variable
Usages Are Allowed, in Strict All Variable Usages Are Allowed we can make this
situation impossible via validation.

In the example below, the variable `$booleanArg` must be non-null because it is
used in the non-null argument (`nonNullBooleanArg`); however since it provides a
default value the variable need not be defined in the request (it is optional,
but non-nullable).

```graphql example
query booleanArgQueryWithDefault($booleanArg: Boolean! = true) {
arguments {
nonNullBooleanArgField(nonNullBooleanArg: $booleanArg)
}
}
```

In the example below, the variable `$booleanArg` must be non-nullable since it
is used in a non-null position, even though the argument `optionalBooleanArg`
has a default value.

```graphql example
query booleanArgQueryWithDefault($booleanArg: Boolean!) {
arguments {
optionalNonNullBooleanArgField(optionalBooleanArg: $booleanArg)
}
}
```

The default value of `optionalBooleanArg` cannot be used when the argument is
specified (either with a literal or a variable). You may choose to copy the
argument's default value to the variable definition to make the variable
optional (but still non-nullable) as in the example below:

```graphql example
query booleanArgQueryWithDefault($booleanArg: Boolean! = false) {
arguments {
optionalNonNullBooleanArgField(optionalBooleanArg: $booleanArg)
}
}
```

#### Legacy All Variable Usages Are Allowed

Implement this only if you are not implementing
[Strict All Variable Usages Are Allowed](#sec-Strict-All-Variable-Usages-Are-Allowed).

**Formal Specification**

- For each {operation} in {document}:
- Let {variableUsages} be all usages transitively included in the {operation}.
- For each {variableUsage} in {variableUsages}:
- Let {variableName} be the name of {variableUsage}.
- Let {variableDefinition} be the {VariableDefinition} named {variableName}
defined within {operation}.
- {IsVariableUsageAllowedLegacy(variableDefinition, variableUsage)} must be
{true}.

IsVariableUsageAllowedLegacy(variableDefinition, variableUsage):

- Let {variableType} be the expected type of {variableDefinition}.
- Let {locationType} be the expected type of the {Argument}, {ObjectField}, or
{ListValue} entry where {variableUsage} is located.
Expand All @@ -1883,25 +2041,6 @@ IsVariableUsageAllowed(variableDefinition, variableUsage):
- Return {AreTypesCompatible(variableType, nullableLocationType)}.
- Return {AreTypesCompatible(variableType, locationType)}.

AreTypesCompatible(variableType, locationType):

- If {locationType} is a non-null type:
- If {variableType} is NOT a non-null type, return {false}.
- Let {nullableLocationType} be the unwrapped nullable type of {locationType}.
- Let {nullableVariableType} be the unwrapped nullable type of {variableType}.
- Return {AreTypesCompatible(nullableVariableType, nullableLocationType)}.
- Otherwise, if {variableType} is a non-null type:
- Let {nullableVariableType} be the nullable type of {variableType}.
- Return {AreTypesCompatible(nullableVariableType, locationType)}.
- Otherwise, if {locationType} is a list type:
- If {variableType} is NOT a list type, return {false}.
- Let {itemLocationType} be the unwrapped item type of {locationType}.
- Let {itemVariableType} be the unwrapped item type of {variableType}.
- Return {AreTypesCompatible(itemVariableType, itemLocationType)}.
- Otherwise, if {variableType} is a list type, return {false}.
- Return {true} if {variableType} and {locationType} are identical, otherwise
{false}.

**Explanatory Text**

Variable usages must be compatible with the arguments they are passed to.
Expand Down Expand Up @@ -2006,3 +2145,26 @@ query booleanArgQueryWithDefault($booleanArg: Boolean = true) {

Note: The value {null} could still be provided to such a variable at runtime. A
non-null argument must raise a _field error_ if provided a {null} value.

#### Are Types Compatible

Both versions of the algorithm share the definition of {AreTypesCompatible()}:

AreTypesCompatible(variableType, locationType):

- If {locationType} is a non-null type:
- If {variableType} is NOT a non-null type, return {false}.
- Let {nullableLocationType} be the unwrapped nullable type of {locationType}.
- Let {nullableVariableType} be the unwrapped nullable type of {variableType}.
- Return {AreTypesCompatible(nullableVariableType, nullableLocationType)}.
- Otherwise, if {variableType} is a non-null type:
- Let {nullableVariableType} be the nullable type of {variableType}.
- Return {AreTypesCompatible(nullableVariableType, locationType)}.
- Otherwise, if {locationType} is a list type:
- If {variableType} is NOT a list type, return {false}.
- Let {itemLocationType} be the unwrapped item type of {locationType}.
- Let {itemVariableType} be the unwrapped item type of {variableType}.
- Return {AreTypesCompatible(itemVariableType, itemLocationType)}.
- Otherwise, if {variableType} is a list type, return {false}.
- Return {true} if {variableType} and {locationType} are identical, otherwise
{false}.