graphql-spec icon indicating copy to clipboard operation
graphql-spec copied to clipboard

allow unions to include interfaces and unions

Open yaacovCR opened this issue 2 years ago • 3 comments

Complements #939 Addresses #711

Similar to #939, this PR expands the robustness of the type system by allowing types that actually fulfill interfaces to be recognized as such by the GraphQL type system.

interface CloningInfo {
  ...
}

union CowOrWolf implements Animal = Cow | Wolf  # allowed by #939
union CowOrCloningInfo = Cow | CloningInfo  # allowed by this PR, note that CloningInfo is an interface
union WolfOrCloningInfo = Wolf | CloningInfo # allowed by this PR, note that CloningInfo is an interface

# note that here we are marking unions explicitly as included within a union.
# Adding a type to the CowOrWolf union will automatically add it to the ParentUnion
# We could also consider adding a constraint on the union definition, see below discussion
union Parent = CowOrCloningInfo | WolfOrCloningInfo | CowOrWolf | Cow | Wolf | CloningInfo

interface Animal {
  parent: Parent
}

type Cow implements Animal {
    parent: CowOrCloningInfo  # unlocked by this PR
}

type Wolf implements Animal{
    parent: WolfOrCloningInfo # unlocked by this PR
} 

Unions

With regard to unions, the goal is to explicitly mark some unions as members of other unions. We have two alternatives:

(A). Let unions include unions as members, as shown above. We could (or could not) require that all members of the unions also be listed (similar to how interfaces implementing child interfaces are required to explicitly list the parent.

Pro:

  1. Simple to reason about, fits with how unions currently work. Con:
  2. Could lead to some automatic behavior, when adding a type to a union that is part of a union, the type gets added to multiple unions. This is ameliorated by requiring all child union members to be explicitly listed.
  3. Union definitions could start to get pretty long if all the combinations must be listed therein.

(B) Add an additional optional constraint on the union requiring all of the members to be members of some other union, similar to how we have resolved #939.

Potential Syntax:

union CowOrWolf implements Animal, subtypes Parent = Cow | Wolf 
union CowOrCloningInfo subtypes Parent = Cow | CloningInfo 
union WolfOrCloningInfo subtypes Parent = Wolf | CloningInfo

union Parent = Cow | Wolf | CloningInfo
# cf
# union Parent = CowOrCloningInfo | WolfOrCloningInfo | CowOrWolf | Cow | Wolf | CloningInfo

Interfaces

For interfaces that are members of unions, it would not seem to make sense to require all the implementations of the interfaces to be listed independently. The whole point is that it is often just as useful to indicate that several interfaces might be returned as it is that several individual member types might be returned. For unions, we also have potentially multiple layers of nesting (unions of unions of unions) for which it would be extremely helpful to require the individual member types to be listed (or to use the second syntax above) while we don't have the same issue with interfaces.

yaacovCR avatar May 27 '22 07:05 yaacovCR

Deploy Preview for graphql-spec-draft ready!

Name Link
Latest commit d8e52f0794423b32874e8c6972211e3dcf5e027f
Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/62e200650ec7d60008f9f9d5
Deploy Preview https://deploy-preview-950--graphql-spec-draft.netlify.app/draft
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] avatar May 27 '22 07:05 netlify[bot]

Deploy Preview for graphql-spec-draft failed.

Name Link
Latest commit 18e70e5e9dd21f533c2e08cd3ad6a51154c495dd
Latest deploy log https://app.netlify.com/sites/graphql-spec-draft/deploys/62907cf3e15503000982fb5a

netlify[bot] avatar May 27 '22 07:05 netlify[bot]

Now implemented in https://github.com/graphql/graphql-js/pull/3682

I added requirement that all subtypes of included abstract types must be explicitly includes within the union. This can always be relaxed later.

yaacovCR avatar Jul 28 '22 01:07 yaacovCR