Skip to content

union does not allow sub-union or covariance #711

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.

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

Open
VladimirAlexiev opened this issue Apr 20, 2020 · 4 comments
Open

union does not allow sub-union or covariance #711

VladimirAlexiev opened this issue Apr 20, 2020 · 4 comments

Comments

@VladimirAlexiev
Copy link

VladimirAlexiev commented Apr 20, 2020

In RDF it's quite common to use subclasses, and somewhat common to use union types (eg schema:rangeIncludes states a field can take values from several different classes; owl:unionOf does a similar thing).

RDF is based on formal logic, so it considers classes (types) as sets. In this interpretation, it's clear how to determine subtypes (especially useful for field covariance). Eg

interface A {}
interface B {}
interface C {}
type A1 implements A {}
type B1 implements B {}
type C1 implements C {}
union U1=A|B|C
union U2=A1|B1|C1   # subtype of U1 because it uses subclasses
union U3=A|B        # subtype of U1 because it uses fewer union members
union U4=A|A1|B|C   # equivalent to U1 because A subsumes A1
union U5=A1|B1      # subtype of U1, U2 and U3

But the spec says:

This means that U1, U3, U4 are not allowed, and neither of them can be used in an object type field.

This creates severe limitations on what polymorphic constructs (in particular RDF constructs) can be mapped to GraphQL.

(I searched here for "union but not 'input union'" and came up only with #518 as a possibly related issue. cc @derek-miller @mavilein)

@evgen4uk
Copy link

evgen4uk commented Dec 21, 2021

Unfortunately, there are not many discussions here but as for me, support of covariance is very crucial.
Let me show a simple example, I will start with schema without union and then I will add unions

interface Animal {
    parent: Animal
}

type Cow implements Animal {
    parent: Cow
}

type Wolf implements Animal{
    parent: Wolf
}

In the above example, we see that parent of a cow can be only a cow, not any animal.
Now let's add a union, for this purpose I will add logic that animals can be cloned, so technically he doesn't have a parent, just information about cloning

type CloningInfo
union AnimalParentOrCloninginfo = Cow | Wolf | CloningInfo

It may be not really a good example, but I just want to show that field parent can return different types

For better reading of this issue, I will use "anonymous" unions and will define them on the field definition

interface Animal {
    parent: Cow | Wolf | CloningInfo
}

type Cow implements Animal {
    parent: Cow | Wolf | CloningInfo
}

type Wolf implements Animal{
    parent: Cow | Wolf | CloningInfo
}

And here we have a huge issue, we cannot decrease the list of union types on the implementation. Therefore we cannot define in a schema that Wolf CANNOT be a parent for Cow

It will be good to be able to have something like this:

interface Animal {
    parent: Cow | Wolf | CloningInfo
}

type Cow implements Animal {
    parent: Cow | CloningInfo
}

type Wolf implements Animal{
    parent: Wolf | CloningInfo
} 

I will appreciate any comments or suggestions on how to deal with this.

@tobiasBora
Copy link

Any news on this? This issue has been around for 4 years, with a PR during 2 years, and forbidding unions of unions create massive code duplication, possible errors, maintainability headaches, and poor readability. Consider for instance:

type QuestionMultipleChoice {
  question: String
  nbChoices: Int!
}

type QuestionText {
  question: String
}

type QuestionNumber {
  question: String
}

type WrongToken {
  _: Boolean
}

union Question = QuestionMultipleChoice | QuestionText | QuestionNumber

# What I'd like (or ideally directly in the Mutation part… but that is another fight):
# union NewQuestionResult = Question | WrongToken
# What I need to write, **for each function that returns different errors**
union NewQuestionResult =  QuestionMultipleChoice | QuestionText | QuestionNumber| WrongToken

type Mutation {
  newQuestion: NewQuestionResult
}

type Query {
  hello: String
}

@yaacovCR
Copy link
Contributor

There are a few related feature requests related to expanding subtyping. A few years ago, I looked into championing some of these efforts, but quickly simply found that I hit more complexity than I had bargained for.

So this work is looking for a new champion! Anyone interested in tackling it might want to take a look at https://github.com/graphql/graphql-wg/blob/main/rfcs/ExpandingSubtyping.md — where I summarized my progress and documented some potential pitfalls. Hope this helps!

@tobiasBora
Copy link

Thanks for your attempts.

So after a quick look, I would (warning: non specialist here):

  • Unions Subtyping Interfaces: I find the implements syntax a bit nicer, but I don't know if it is harder to parse or not, if yes maybe just using directives would be a first start. Regarding the question, I would actually do a mix of both (like what is possible right now with __typename that can be factored out of the ... on Foo). Basically, the second lengthy syntax should always work, but when a type is the same in all elements, I think we should allow to write it "in front" without copy/pasting it on all elements of the list for readability and maintainability.
  • Unions Subtyping Unions: For me the only viable option is Option A. All other solutions require duplicating code, and it is also what I'd write naturally.
  • Interfaces Subtyping Unions: With the same argument, also only option A applies, otherwise we are again back to a maintainability headache for the developpers that must spend their time copy/pasting lists of definitions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants