OverlappingFieldsCanBeMerged throws an error when querying same field w/ and w/o attributes for different union types

[Orrison]

This is a well-known/discussed issue within the GraphQL community and much discussed here: https://github.com/graphql/graphql-js/issues/53

If querying the same field for different union types has the same name but different Type definitions, this should be possible without an alias. But it seems we are currently restricting that within the graphql-php implementation.

[spawnia]

We mostly do what graphql-js does. Can you find a test case for this in https://github.com/graphql/graphql-js/blob/main/src/validation/tests/OverlappingFieldsCanBeMergedRule-test.ts or https://github.com/webonyx/graphql-php/blob/master/tests/Validator/OverlappingFieldsCanBeMergedTest.php?

[oprypkhantc]

Just stumbled upon this and it makes absolutely no sense. Somehow, the issue on graphql-js side is closed 🙈

@spawnia This is the relevant test case: https://github.com/webonyx/graphql-php/blob/master/tests/Validator/OverlappingFieldsCanBeMergedTest.php#L799

In our case, I'm trying to query a field on a union of two types, one of which happens to have it nullable, and the other doesn't:

union UnionUserLead = Lead | User

type Lead {
    email: EmailAddress
    id: ID!
    name: String
    phone: PhoneNumber
}

type Subscription {
    messageReceived: ThreadMessage!
}

type ThreadParticipant {
    id: ID!
    owner: UnionUserLead!
}

type ThreadMessage {
    body: String
    id: ID!
    sender: ThreadParticipant!
}

type User {
    id: ID!
    name: String!
}

subscription {
    messageReceived {
        id
        sender {
            id
            owner {
                __typename
                ... on User {
                    id
                    name
                }
                ... on Lead {
                    id
                    name
                }
            }
        }
        body
    }
}

which results in the error: Fields "name" conflict because they return conflicting types String! and String. Use different aliases on the fields to fetch both if this was intentional.

The client can already differentiate between different types through __typename, and definitely should be doing so, regardless of whether types of fields overlap or not.

[oprypkhantc]

Overwriting the rule's validation like so works:

class CustomOverlappingFieldsCanBeMerged extends OverlappingFieldsCanBeMerged
{
	protected string $name = OverlappingFieldsCanBeMerged::class;

	protected function findConflict(QueryValidationContext $context, bool $parentFieldsAreMutuallyExclusive, string $responseName, array $field1, array $field2): ?array
	{
		[$parentType1] = $field1;
		[$parentType2] = $field2;

		$areMutuallyExclusive = $parentFieldsAreMutuallyExclusive
			|| (
				$parentType1 !== $parentType2
				&& $parentType1 instanceof ObjectType
				&& $parentType2 instanceof ObjectType
			);

		if ($areMutuallyExclusive) {
			return null;
		}

		return parent::findConflict($context, $parentFieldsAreMutuallyExclusive, $responseName, $field1, $field2);
	}
}

DocumentValidator::addRule(new CustomOverlappingFieldsCanBeMerged());

Based on the test case, it does work as intended.

[spawnia]

What does the latest edition of the GraphQL specification say about this?

[oprypkhantc]

The spec says it should indeed result in an error: https://spec.graphql.org/draft/#example-54e3d

I'll ask about this in https://github.com/graphql/graphql-spec, although I definitely don't have the capacity to present an RFC to change this. For us, it seems that the solution would still be an overwritten rule.