[RFC] Union types can implement interfaces

[derek-miller]

Problem
The spec, as it is currently, is ambiguous when defining how a union type should be handled when used to fulfill an interface.

An object field type is a valid sub‐type if it is an Object type and the interface field type is either an Interface type or a Union type and the object field type is a possible type of the interface field type.

Due to this, implementations of the spec do not allow the use of a union to fulfill an interface even though all members implements said interface. See issue graphql-js #1488.

Solution
A union should be considered an implementation of an interface if and only if each member type in the union implements said interface.

Examples
Consider the following schema:

type Query {
  test: MyConnection
}

interface Node {
  id: ID
}

interface Connection {
  nodes: [Node]
}

type NodeA implements Node {
  id: ID
}
type NodeB implements Node {
  id: ID
}
union MyNode =
  | NodeA
  | NodeB

type MyConnection implements Connection {
  nodes: [MyNode]
}

The above schema fails validation as MyNode does not directly implement Node even though all members do.

Concerns
I'm not aware of any concerns if this change was approved as it is additive and fixes an ambiguity in the spec. Existing schema would remain unaffected.

There may be issues unknown to me due to my incomplete understanding of the entire spec, so let me know if I missed anything.

[rmosolgo]

🤔 As a library author, I'm interested in keeping the language as simple as possible. In noticed an interesting mismatch between interface Node and union MyNode. In this trivial example, wouldn't it be a suitable solution to change union MyNode to be interface MyNode? Then what ever shared fields you needed to express could be added to MyNode.

Or, are there other cases where using an interface instead isn't a suitable solution?

[derek-miller]

@rmosolgo That would not work in the case where you have many implementations of your proposed MyNode interface, but only wanted to support returning a subset of those in a particular connection. The schema that I am working on where I hit this limitation has ~20 nodes and only a few of them are available at a time through a connection interface.

[rmosolgo]

Oh, so you want to specify a subset of objects which might be present at a certain point in the query, similar to a union? How about using several different interfaces, one for each applicable scenario? For example, interface MyNodeFromXConnection (where XConnection is a "particular connection" with a subset of objects as its possible return types). Then, objects could implement each applicable type: type X implements MyNode, MyNodeFromXConnection vs type Y implements MyNode (but not the XConnection-specific one). That would give you specific subsets for certain contexts.

It moves the complexity of this use case from the language specification to the application, but it might be a way to test the viability of your proposal!

Even if my imagined implementation is wrong, is that the kind of behavior you're looking for?

[leebyron]

I noticed that in your example, Connection is not being used anywhere, and removing it should result in a valid schema:

type Query {
  test: MyConnection
}

interface Node {
  id: ID
}

type NodeA implements Node {
  id: ID
}
type NodeB implements Node {
  id: ID
}
union MyNode =
  | NodeA
  | NodeB

type MyConnection {
  nodes: [MyNode]
}

Is this sufficient to solve your problem? Or is there a circumstance where a field returns a Connection interface type directly and not a specific Object type?

[derek-miller]

Our schema is rather large and we have several connection types. The purpose I originally had for the Connection interface is to enforce consistency across them. With that said, I do not want to stress my example and find alternative solutions in this issue, but rather have a discussion about how the spec should define this situation. I believe the spec should definitely state how this should be handled so there is no ambiguity or incorrect implementations.

[derek-miller]

Also I want to point out that I hit the same issue when having an Edge interface as so:

interface Connection {
  nodes: [Node]
  edges: [Edge]
}

interface Edge {
  node: Node
  # cursor, textMatches, etc
}

Again, @leebyron 's workaround will work if we remove the Edge interface, but pretty soon you start losing the power of a type system.

[leebyron]

Another suggestion would be to move the union to the connection types instead of the node types:

union MyConnection =
  | ConnectionA
  | ConnectionB

type ConnectionA implements Connection {
  nodes: [NodeA]
}

type ConnectionB implements Connection {
  nodes: [NodeB]
}

The reason for the invalidation is because union types can expand over time. While the members of the union type may happen to each be valid, future evolution of that union type may later cause an inconsistency.

If this continues to be limiting, another suggestion for an RFC would be to attempt to change the schema validation rules so that each member of the union is compared to the expected interface type rather than the union itself being compared. In other words, a union MyNode can be said to implement interface Node IFF each member type in MyNode implements interface Node.

[derek-miller]

a union MyNode can be said to implement interface Node IFF each member type in MyNode implements interface Node

This is exactly what I was suggesting graphql-js #1488, but it was declined because the spec did not explicitly cover this case and they suggested I open an issue here first.

[leebyron]

Great - I'd suggest taking a look at https://github.com/facebook/graphql/blob/master/CONTRIBUTING.md#contributing-to-graphql-libraries which is hopefully helpful in guiding you towards next steps if you'd like to champion this effort

[derek-miller]

I am willing to champion this change. Let me know what I need to do to progress this to the proposal stage.

[derek-miller]

@leebyron I updated the issue description. Let me know what the next steps are. It is not clear from the contributing file if the champion is/should be the one to open the PR with the changes to the spec. I can make an attempt at it if necessary.

[derek-miller]

@leebyron Any update on how to move this forwards?

[derek-miller]

@leebyron, Can I get an update on this issue?

[leebyron]

Sorry for missing the previous comment. I believe this is just waiting for a proposal from a champion. The document I linked in a prior comment should help guide you if you want to do that, and you can look at some of the other PRs which have progressed through the stages.

I think I viable proposal would need to answer questions about edge cases, especially for how schema evolve over time and how validation rules need to change.

I think it’s also helpful to prototype in graphql.js to get a better sense of what’s affected

[AnthonyMDev]

I'm a big fan of allowing unions to implement interfaces.

Solution
A union should be considered an implementation of an interface if and only if each member type in the union implements said interface.

The issue I see with this proposal is that if, in the future, a new member type is added to the union that does not implement the interface, everything breaks down here. As stated above by @leebyron:

The reason for the invalidation is because union types can expand over time. While the members of the union type may happen to each be valid, future evolution of that union type may later cause an inconsistency.

What I'd like to propose is that we allow unions to actually implement an interface explicitly. Creating a contract that all future additions to the union must implement the interface.

union MyNode implements Node =
  | NodeA
  | NodeB

[connorjs]

A couple points as I have hit this issue.

1. Context: Defining Connection, Edge, etc as part of Relay-style pagination

2. Motivation: My Node type has union with error results following 200 OK! Error Handling in GraphQL blog post by Sasha Solomon

3. Proposal: I had the same thought as @AnthonyMDev - I love that suggestion of using the implements phrase!

Does anyone have recommendations/workarounds to share on how they model this?

[yaacovCR]

Spec PR: #939
graphql-js PR: graphql/graphql-js#3527

[yaacovCR]

I am taking a stab at this via new intersection type, see new graphql-js PR @ graphql/graphql-js#3550

instead of:

union SomeUnion implements SomeInterface = TypeA | TypeB
interface SomeInterface {
  someField: String
}
typeA implements SomeInterface {
  someField: String
}
typeB implements SomeInterface {
  someField: String
}

I am suggesting:

intersection SomeIntersection = SomeUnion & SomeInterface
union SomeUnion = TypeA | TypeB
interface SomeInterface {
  someField: String
}
typeA implements SomeInterface {
  someField: String
}
typeB implements SomeInterface {
  someField: String
}

[rivantsov]

in my opinion, confusing as hell, this intersection thing. Seriously, after reading spec PR, I don't get it, completely, what is that

[yaacovCR]

An intersection is a higher-order abstract type. It is similar to the other abstract types, interfaces and unions, in that an intersection can be fulfilled by any number of object types. Intersections are defined by a set of constraining abstract types and include only the object types that fulfill all of those abstract types, i.e. are members of all of the intersection's unions and implement all of the intersection's interfaces.

In the motivational example, an intersection would includes a union and an interface, and so the intersection will only include the types within the union that implement the interface. It's a different way of achieving "unions that implement interfaces" through a separate type.

I agree that the spec PR probably needs to give a better example to show the motivation. I'll try to work on that.

[yaacovCR]

For the alternative to unions implementing interfaces, I am posting here links to spec PR that @rivantsov mentioned above and link to discussion for further thoughts.
spec PR: #941
discussion: graphql/graphql-wg#944

[yaacovCR]

I agree that the spec PR probably needs to give a better example to show the motivation. I'll try to work on that.

I updated the linked spec PR to include the motivational example. I updated the linked discussion on intersections above to include the motivational example, example syntax originally posted here. I corrected some typos within that syntax.

[yaacovCR]

Summary of discussion from WG https://github.com/graphql/graphql-wg/blob/main/agendas/2022/2022-05-05.md :

union HousePet implements Node was overwhelmingly felt to be best because it is (1) expressive (2) concise (3) less complex (4) will lead to early validation error, which overall would be most valuable. Intersections were felt to be powerful tools that would then propagate a small number of breaking changes to a large number of dangerous and --more importantly-- possibly silent errors!

The action item will then be to move forward as possible with #939 . Immediate next steps is to refine the implementation at graphql/graphql-js#3527 with further checking for all the necessary validation and test changes.