Skip to content
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

define spec for graphql union #459

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

craig-day
Copy link

I took a stab at defining and adding examples for union types. As unions are very similar to interfaces, I based it heavily on the interface setup. I chose to also leverage a java interface as the union itself, because an interface with no defined fields/methods represents the concept of a union well.

Another approach to this could be to not have a "base class", i.e. interface, at all. Instead, the @Union annotation could require the value and then any types annotated with the same union name would become members of that union. For example:

@Union("MyUnion")
public class Foo {
  // ...
}

@Union("MyUnion")
public class Bar {
  // ...
}

would create

type Foo {
  # ...
}

type Bar {
  # ... 
}

union MyUnion = Foo | Bar

Additional Considerations

Mixing polymorphism in GraphQL is not very nice. In the case of unions, a member can only be a type. This means if you want an interface to be a part of a union, you must instead add each concrete implementation of the interface to the union. However, you can then query the union with fragments as if it were an interface.

Should the spec explicitly define what should happen in this scenario? If someone structures code where a union would contain members that are interfaces, should it be explicitly stated that the implementation must automatically expand the implementations? For example:

@Union
public interface MyUnion {
}

@Interface
public interface MyInterface extends MyUnion {
  String getName();
}

public class Foo implements MyInterface {
  // ...
}

public class Bar implements MyInterface {
  // ...
}

should generate

interface MyInterface {
  name: String
}

type Foo implements MyInterface {
  name: String
}

type Bar implements MyInterface {
  name: String
}

union MyUnion = Foo | Bar

Obviously this example is contrived, but is just to illustrate the scenario. I have a few services with very large schemas (10k+ types) and we have plenty of situations where a union contains one or more interfaces.

@phillip-kruger
Copy link
Member

Hi @craig-day - thanks for this. We typically do things in an implementation first, to give it some bake time, before we move it to the spec. Maybe we should attempt this in smallrye graphql first?

@craig-day
Copy link
Author

@phillip-kruger thanks for the reply. I'm working on this implementation in smallrye too! I'll cross-link a PR here once I get that worked through.

@phillip-kruger
Copy link
Member

@craig-day - Great. You can move all this (api) stuff to SmallRye until it's ready to move up to the spec. (see https://github.com/smallrye/smallrye-graphql/tree/main/server/api)

@craig-day
Copy link
Author

@phillip-kruger smallrye impl PR here: smallrye/smallrye-graphql#1469

@phillip-kruger
Copy link
Member

@craig-day Thanks. Let discuss it in SmallRye, get it merged there and into a runtime, to get some bake time, before we get back here. Maybe convert this to a draft ?

@craig-day craig-day marked this pull request as draft July 26, 2022 15:41
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

Successfully merging this pull request may close these issues.

2 participants