graphql / graphql/graphql-spec
Relax the validation of field selection on mutually exclusive types
- Dominant language
- JavaScript
- Stars
- 14.6k
- Forks
- 1.2k
- PR merge metrics
- No merged PRs in 30d
Description
Hey.
Currently, [per spec, paragraph 5.3.2](https://spec.graphql.org/draft/#sec-Field-Selection-Merging), two fields of the same name cannot be selected on two different, mutually exclusive types, if field types or arguments differ. E.g. given this schema:
```graphql
union CatOrDog = Cat | Dog
type Query {
findPet: CatOrDog!
findDog: Dog!
}
interface Pet {
name: String!
}
type Dog implements Pet {
name: String!
nickname: String
barkVolume: Int
anotherCommonlyNamedField: String
}
type Cat implements Pet {
name: String!
nickname: String
meowVolume: Int
anotherCommonlyNamedField: String!
}
```
this will result in an error ([Example #125 in the spec](https://spec.graphql.org/draft/#example-54e3d)):
```graphql
fragment conflictingDifferingResponses on Pet {
... on Dog {
someValue: nickname
}
... on Cat {
someValue: meowVolume
}
}
```
and so will this:
```graphql
query {
findPet {
__typename
... on Dog {
name
anotherCommonlyNamedField
}
... on Cat {
name
anotherCommonlyNamedField
}
}
}
```
due to `anotherCommonlyNamedField` field having different return types in `Dog` and `Cat` types.
---
It forces usage of aliases when objects are obtained through a union type. E.g. if client's uses a type, based on structure of a GraphQL type `Dog`, one they can obtain from calling `findDog`, they would no longer be able to have the same structure if they need to switch to `findPet`.
For example, let's say a client uses this query:
```graphql
{
findDog {
name
nickname
barkVolume
anotherCommonlyNamedField
}
}
```
and they have a type like so, used in multiple places in the project:
```typescript
type Dog = {
name: string
nickname: string | null
barkVolume: number | null
anotherCommonlyNamedField: string | null
}
```
and then `Cat` and `findPet` are introduced into the schema, the client needs to support both `Cat` and `Dog`, they'd need to switch to the new `findPet` field:
```graphql
{
findPet {
__typename
... on Dog {
name
nickname
barkVolume
anotherCommonlyNamedField
}
... on Cat {
name
nickname
meowVolume
anotherCommonlyNamedField
}
}
}
```
which, unexpectedly, doesn't work. Instead, they have to introduce aliases:
```graphql
{
findPet {
__typename
... on Dog {
name
nickname
barkVolume
anotherCommonlyNamedFieldNullable: anotherCommonlyNamedField
}
... on Cat {
name
nickname
meowVolume
anotherCommonlyNamedFieldNonNullable: anotherCommonlyNamedField
}
}
}
```
which breaks the original `Dog` interface the client had. Now, they either have to manually convert the type to a different structure (rename `anotherCommonlyNamedFieldNullable` and `anotherCommonlyNamedFieldNonNullable` back into `anotherCommonlyNamedField`), or change it across the entire client code.
This is suboptimal, as it forces the client to do extra work. In either situation, the client **must** check for the typename through `__typename` - because only dogs have `barkVolume`, and only cats have `meowVolume`. If this check is done either way, why would different types of a shared-name field matter?
In other words, if the client already has to differentiate between different, unrelated, mutually exclusive types, that may have entirely different number of fields and an entirely different structure, why do only fields that intersect between the two have to have a shared type? If response structure is expected to be the same, regardless of the actual type of an object, then shouldn't it also enforce that a `Dog` must have a `meowVolume`, and a `Cat` must have a `barkVolume`?
## Reported elsewhere
This has been reported as a bug/problem in implementations of GraphQL:
- JS: https://github.com/graphql/graphql-js/issues/53
- PHP: https://github.com/webonyx/graphql-php/issues/1500
- Java: https://github.com/graphql-java/graphql-java/issues/1281
- C#: https://github.com/ChilliCream/graphql-platform/issues/2240
- ... probably others
The issues were closed as the spec dictates the specific behaviour a GraphQL server should follow.
## Proposal
The proposal is to skip conflict checking for mutually exclusive types altogether. In terms of `graphql-js` implementation, a patch would look like so:
https://github.com/graphql/graphql-js/blob/v16.11.0/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts#L606
```diff
- if (!areMutuallyExclusive) {
+ if (areMutuallyExclusive} {
+ return null;
+ }
```
An alternative proposal is to do the same, but only **if** `__typename` field was requested.
Contributor guide
Assessment
This issue has not been assessed yet.