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

[RFC] GraphQL Input Union type #488

Closed
frikille opened this issue Aug 2, 2018 · 75 comments

Comments

@frikille
Copy link

@frikille frikille commented Aug 2, 2018

[RFC] GraphQL Input Union type

Background

There have been many threads on different issues and pull requests regarding this feature. The main discussion started with the graphql/graphql-js#207 on 17/10/2015.

Currently, GraphQL input types can only have one single definition, and the more adoption GraphQL gained in the past years it has become clear that this is a serious limitation in some use cases. Although there have been multiple proposals in the past, this is still an unresolved issue.

With this RFC document, I would like to collect and summarise the previous discussions at a common place and proposals and propose a new alternative solution.

To have a better understanding of the required changes there is a reference implementation of this proposal, but that I will keep up to date based on future feeback on this proposal.

The following list shows what proposals have been put forward so far including a short summary of pros and cons added by @treybrisbane in this comment

  • __inputname field by @tgriesser

    RFC document: #395

    Reference implementation: graphql/graphql-js#1196

    This proposal was the first official RFC which has been discussed at the last GraphQL Working Group meeting.
    This proposal in this current form has been rejected by the WG because of the __inputname semantics. However, everyone agrees that alternative proposals should be explored.

    • Pros:
      • Expresses the design intent within the schema
      • Supports unions of types with overlapping fields
      • Removes the costs of the tagged union pattern (both to the schema and field resolution)
      • Addresses the feature asymmetry of unions within the type system
    • Cons:
      • Adds complexity to the language in the form of input union-specific syntax
      • Adds complexity to the language in the form of additional validation (around __inputtype, etc)
      • Adds complexity to the request protocol in the form of a (input union-specific) constraint
  • Tagged union by @leebyron and @IvanGoncharov

    Original comment

    input MediaBlock = { post: PostInput! } | { image: ImageInput! }
    • Pros:
      • Expresses the design intent within the schema
      • Removes the costs of the tagged union pattern (both to the schema and field resolution)
      • Does not require changes to the request protocol
    • Cons:
      • No support for unions of types with overlapping fields
      • Introduces inconsistencies between the syntax for output unions and the syntax for input unions
      • Adds complexity to the language in the form of input union-specific syntax
      • Adds complexity to the language in the form of additional validation (around enforcing the stipulations on overlapping fields, nullability, etc)
      • Does not fully address the feature asymmetry of unions within the type system
  • Directive

    Original comment

    input UpdateAccountInput @inputUnion {
      UpdateAccountBasicInput: UpdateAccountBasicInput
      UpdateAccountContactInput: UpdateAccountContactInput
      UpdateAccountFromAdminInput: UpdateAccountFromAdminInput
      UpdateAccountPreferencesInput: UpdateAccountPreferencesInput
    }
    • Pros:
      • Requires no language or request prototcol changes beyond a new directive
      • Supports unions of types with overlapping fields
    • Cons:
      • Adds complexity to the language in the form of a new directive
      • Does not express the design intent within the schema (the presence of a directive completely changes the meaning of a type definition which would otherwise be used to describe a simple object type)
      • Does not remove the costs of the tagged union pattern
      • Does not address the feature asymmetry of unions within the type system

Proposed solution

Based on the previous discussions I would like to propose an alternative solution by using a disjoint (or discriminated) union type.

Defining a disjoint union has two requirements:

  1. Have a common literal type field - called the discriminant
  2. A type alias that takes the union of those types - the input union

With that our GraphQL schema definition would be the following (Full schema definition)

literal ImageInputKind
literal PostInputKind

input AddPostInput {
  kind: PostInputKind!
  title: String!
  body: String!
}

input AddImageInput  {
  kind: ImageInputKind!
  photo: String!
  caption: String
}

inputUnion AddMediaBlock = AddPostInput | AddImageInput

input EditPostInput {
  inputTypeName: PostInputKind!
  title: String
  body: String
}

input EditImageInput {
  inputTypeName: ImageInputKind!
  photo: String
  caption: String
}

inputUnion EditMediaBlock = EditPostInput | EditImageInput

type Mutation {
  createMedia(content: [AddMediaBlock]!): Media   
  editMedia(content: [EditMediaBlock]!): Media
}

And a mutation query would be the following:

mutation {
  createMedia(content: [{
    kind: PostInputKind
    title: "My Post"
    body: "Lorem ipsum ..."
  }, {
    kind: ImageInputKind
    photo: "http://www.tanews.org.tw/sites/default/files/imce/admin/user2027/6491213611_c4fc290a33_z.jpg"
    caption: "Dog"
  }]) {
    content {
      ... on PostBlock {
        title
        body
      }
      ... on ImageBlock {
        photo
        caption
      }
    }
  }
}

or with variables

mutation CreateMediaMutation($content: [AddMediaBlock]!) {
  createMedia(content: $content) {
    id
    content {
      ... on PostBlock {
        title
        body
      }
      ... on ImageBlock {
        photo
        caption
      }
    }
  }
}
{
  "content": [{
    "kind": "PostInputKind",
    "title": "My Post",
    "body": "Lorem ipsum ...",
  }, {
    "kind": "ImageInputKind",
    "photo": "http://www.tanews.org.tw/sites/default/files/imce/admin/user2027/6491213611_c4fc290a33_z.jpg",
    "caption": "Dog"
  }]
}

New types

Literal type

The GraphQLLiteralType is an exact value type. This new type enables the definition a discriminant field for input types that are part of an input union type.

Input union type

The GraphQLInputUnionType represent an object that could be one of a list of GraphQL Input Object types.

Input Coercion

The input union type needs a way of determining which input object a given input value correspond to. Based on the discriminant field it is possible to have an internal function that can resolve the correct GraphQL Input Object type. Once it has been done, the input coercion of the input union is the same as the input coercion of the input object.

Type Validation

Input union types have a potential to be invalid if incorrectly defined:

  • An input union type must include one or more unique input objects
  • Every included input object type must have:
    • One common field
    • This common field must be a unique Literal type for every Input Object type

Using String or Enum types instead of Literal types

While I think it would be better to add support for a Literal type, I can see that this type would only be useful for Input unions; therefore, it might be unnecessary. However, it would be possible to use String or Enum types for the discriminant field, but in this case, a resolveType function must be implemented by the user. This would also remove one of the type validations required by the input type (2.2 - The common field must be a unique Literal type for every Input Object type).

Final thoughts

I believe that the above proposal addresses the different issues that have been raised against earlier proposals. It introduces additional syntax and complexity but uses a concept (disjoint union) that is widely used in programming languages and type systems. And as a consequence I think that the pros of this proposal outweigh the cons.

  • Pros

    • Expresses the design intent within the schema
    • The discriminant field name is configurable
    • Supports unions of types with overlapping fields
    • Addresses the feature asymmetry of unions within the type system
    • Does not require changes to the request protocol
  • Cons

    • Adds complexity to the language in the form of literal-specific syntax (might not need)
    • Adds complexity to the language in the form of input union-specific syntax
    • Adds complexity to the language in the form of additional validations (input union, discriminant field, input union resolving (might not need))

However, I think there are many questions that needs some more discussions, until a final proposal can be agreed on - especially around the new literal type, and if it is needed at all.

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Aug 2, 2018

Currently, GraphQL input types can only have one single definition, and the more adoption GraphQL gained in the past years it has become clear that this is a serious limitation in some use cases.

This is the main point of this discussion, instead of proposing competing solutions we should focus on defining a set of problems that we want to address. From previous discussions, it looks like the main use case is "super" mutations. If this is also the case for you RFC should address why you can't do separate createMediaPost or createMediaImage. Do you have any other use real-life use cases that you want to address with this feature?

So this discussion should be driven by real-life user cases and the proposed solution should be presented only in the context of solving this use cases.
IMHO, without a defined set of the use cases, this discussion looks like a beauty contest for SDL syntax and JSON input.

Does not address the feature asymmetry of unions within the type system

Feature asymmetry between input and output types is not an issue since they are fundamentally different in API case.

Output types represent entities from the business domain (e.g. Post, Image, etc.) and because of that, they are usually reused all around the schema.

Input types represent set of data required for a particular action or query (e.g. AddPostInput, EditPostInput) and thus in the majority of cases, input types have very limited reusability.

IMHO, that's why it makes sense to sacrifice some simplicity to allow better expressing your domain entities and facilitate reusability by allowing interfaces and unions on output types.

As for input types instead of the feature-reach type system, we need to provide set of validation constraints, e.g. based on your example this type will satisfy:

type AddMediaBlock {
  kind: PostInputKind!
  title: String
  body: String
  photo: String
  caption: String
}

It's working and fully solve the present problem there are the only two problems:

  • documentation - you need to describe valid field combination in description
  • validation - you need to write validation manually

So instead of creating two additional types and complicating type system with inputUnion we can just solve the above problems by describing required validation constraints: type AddMediaBlock = { ... } | { ... }.

Using String or Enum types instead of Literal types

IMHO, literal is just polute namespace and force to use PostInputKind istead of Post.
We shouldn't mix client data with typenames from typesystem in any shape or form especially since it was one of main arguments against __inputname.

So it should be either in place defined strings or enum values.

@frikille

This comment has been minimized.

Copy link
Author

@frikille frikille commented Aug 2, 2018

From previous discussions, it looks like the main use case is "super" mutations.
Do you have any other use real-life use cases that you want to address with this feature?

Yes. We are building a headless content management platform: users can define their own content types, fields and valiations. They get a GraphQL API that they can use to read and write data. And we want to support a new feature whereby a content type field can have multiple types. So an output type would be something like this:

type PageDocument {
  id: ID
  title: String
  slug: String
  author: Author
  sections: [Section]
}

type Section {
  title: String
  blocks: [SectionBlock]
}

union SectionBlock = ImageGallery | HighlightedImage | ArticleList | RichText

As you can see, this doesn't requires a top level "super" mutation. I agree with you on that a super mutation can and should be just separate mutations. But in our case it is not not possible since the input field that can have multiple types is deeply nested.

An example schema for the input types:

mutation createPageDocument(document: PageDocumentInput!): PageDocument

input PageDocumentInput {
  title: String!
  slug: String!
  author: ID!
  sections: [SectionInput]
}

input SectionInput = {
  title: String!
  blocks: [SectionBlockInput]
}

enum SectionBlockInputEnum = {
  ImageGallery
  HighlightedImage
  Articles
  RichText
}

input ImageGalleryInput = {
  kind: SectionBlockInputTypeEnum!
  title: String!
  description: String!
  imageUrls: [String!]!
}

input HighlightedImageInput {
  kind: SectionBlockInputTypeEnum!
  url: String!
  caption: String!
}

input ArticaleListInput {
  kind: SectionBlockInputTypeEnum!
  articleIds: [ID!]!
}

input RichTextInput {
  kind: SectionBlockInputTypeEnum!
  html: String
  markdown: String
}

I deliberately left out the definition of the SectionBlockInput input type. Based on my proposal it would be this:

inputUnion SectionBlockInput = ImageGalleryInput | HighlightedImageInput | ArticleListInput | RichTextInput

I would skip the "super" SectionBlockInput type definition, that includes every field from the four input types, because as you mentioned that have two problems:

  • documentation: We must be able to generate the best documentation to our users since they can use the API for writing data in to their project
  • validation: this is the bigger issue. We started with GraphQL mostly because of the type system and query validation it provides. And while we can write a validation engine that solves this problem, we feel that it's just another validation should live in GraphQL.

With your required validation constraint suggestion I suppose it would be something like this:

input SectionBlockInput = {  
  kind: SectionBlockInputTypeEnum!
  title: String!
  description: String!
  imageUrls: [String!]!
} | {
  kind: SectionBlockInputTypeEnum!
  url: String!
  caption: String!
} | {
  kind: SectionBlockInputTypeEnum!
  articleIds: [ID!]!
} | {
  kind: SectionBlockInputTypeEnum!
  html: String
  markdown: String
}

I think these two examples are pretty close to each other and would achieve the same result. The main question is that it would be better doing this:

  • with a new type (inputType)
  • or by new validation constraint

IMHO, literal is just polute namespace and force to use PostInputKind istead of Post.
We shouldn't mix client data with typenames from typesystem in any shape or form especially since it was one of main arguments against __inputname.

I accept this.It had a lot of advantages using it when I implemented the input type resolver. I would be happy to drop this one completely as it already felt a bit unnecessary.

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Aug 3, 2018

@frikille Thank you for sharing your use case it helped me a lot with understanding your particular problem.
Is it the only type of scenarios where you need Input Union types?

@frikille

This comment has been minimized.

Copy link
Author

@frikille frikille commented Aug 3, 2018

There are some other places where it would be useful, but those are top level mutations (creating a document) which can be solved other ways. However the input union support would help with that use case as well. Our main problem is that we have completely different schemas for every user project, and it would be better if we were able to use a common mutation for creating a document instead of dynamically changing the mutation name. But I understand that it can be solved without input unions, and we've been using that solution for a long time so for that it is not necessary. However, we don't see how the use case I described in my previous comment can be solved without input unions.

@jturkel

This comment has been minimized.

Copy link

@jturkel jturkel commented Aug 3, 2018

Creating/updating an entity (PageDocument in the example above) with a heterogeneous collection of nested value objects (different types of Section in the example above) is a great use case for input unions. If you're looking for more use cases @xuorig describes batching multiple mutating operations (each of which may have very different structure) into a single atomic mutation in this blog post. The post describes a project board like Trello where a user can add/move/delete/update cards in a single atomic operation. The corresponding SDL looks something like this with input unions:

type Mutation {
  updateBoard(input: UpdateBoardInput!): UpdateBoardPayload!
}

type UpdateBoardInput {
  operations: [Operation!]!
}

union Operation = AddOperation | UpdateOperation |  MoveOperation | DeleteOperation

input AddOperation {
  id: ID!
  body: String!
}

input UpdateOperation {
  id: ID!
  body: String!
}

input MoveOperation {
  id: ID!
  newPos: Int!
}

input DeleteOperation {
  id: ID!
  body: String!
}

@OlegIlyenko also described similar ideas for batching multiple operations into a single atomic mutation in the context of CQRS and commerce in this blog post.

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Aug 3, 2018

@frikille @jturkel Thanks for more examples I will try to study them this weekend.
But from the first glance, it looks like all examples are centered around mutations.

Next major question is how proposed input unions affect code generation especially for strong type languages, e.g. Java. I suspect that such languages don't support unions based on field value as the discriminator. It especially important for mobile clients.
Can someone check how it will affect apollo-ios and apollo-android?

@mjmahone Maybe you know, can something like this affect Facebook codegen for iOS and Android?

@jturkel

This comment has been minimized.

Copy link

@jturkel jturkel commented Aug 4, 2018

@IvanGoncharov - We have examples in the product information management space where input unions would be useful as field arguments. On such example is retrieving a filtered list of products:

input StringEqualityFilter {
  propertyId: String!
  value: String!
}

input NumericRangeFilter {
  propertyId: String!
  minValue: Int!
  maxValue: Int!
}

input HasNoValueFilter {
  propertyId: String!
}

inputunion Filter = StringEqualityFilter | NumericRangeFilter | HasNoValueFilter

type Query {
  products(filters: [Filter!]): ProductCursor
}

Given this schema customers can retrieve a list of products that meet the following conditions:

  • The 'Brand' property is equal to 'Apple'
  • The 'Category' property is equal 'Laptops'
  • The 'Price' property is between 1000 and 2000
  • The 'Description' property has no value

Note our customers can create their own "schema" for modelling product information so our GraphQL schema can't have hardcoded knowledge of things like brands and categories thus the more general propertyId in this example.

@treybrisbane

This comment has been minimized.

Copy link

@treybrisbane treybrisbane commented Aug 5, 2018

@IvanGoncharov I agree that we should try to keep focused on practical use cases for input unions. A really big one is pretty much any mutation that leverages recursive input types.

I'm looking at implementing a GraphQL API for a pattern-matching engine that looks something like this:

type Query {
  patternMatcher: PatternMatcher!
}

type PatternMatcher {
  cases: [Case!]!
}

type Case {
  condition: Condition
  action: Action!
}

union Condition = AndCondition | OrCondition | NotCondition | PropertyEqualsCondition # ...etc

type AndCondition {
  leftCondition: Condition!
  rightCondition: Condition!
}

type OrCondition {
  leftCondition: Condition!
  rightCondition: Condition!
}

type NotCondition {
  condition: Condition!
}

type PropertyEqualsCondition {
  property: String!
  value: String!
}

# ...etc

union Action = # ...you get the idea

Naturally, I need to be able to create new Cases. Ideally, this would look something like this:

type Mutation {
  createCase(input: CreateCaseInput!): CreateCasePayload!
}

input CreateCaseInput {
  caseSpecification: CaseSpecification!
}

type CreateCasePayload {
  case: Case!
}

input CaseSpecification {
  condition: ConditionSpecification
  action: ActionSpecification!
}

inputunion ConditionSpecification = AndConditionSpecification | OrConditionSpecification | NotConditionSpecification | PropertyEqualsConditionSpecification # ...etc

input AndConditionSpecification {
  leftCondition: ConditionSpecification!
  rightCondition: ConditionSpecification!
}

input OrConditionSpecification {
  leftCondition: ConditionSpecification!
  rightCondition: ConditionSpecification!
}

input NotConditionSpecification {
  condition: ConditionSpecification!
}

input PropertyEqualsConditionSpecification {
  property: String!
  value: String!
}

# ...etc

inputunion ActionSpecification = # ...again, you get the idea

Describing this properly without input unions is impossible. The current workaround I'm using is the "tagged-union pattern", however in my experience, use of that pattern is continually met with confusion and distaste due to its verbosity and lack of clear intent.

Note that your suggestion of input Condition = { ... } | { ... } really doesn't scale here. The ability to name the branches of the input union is pretty much necessary to maintain readability in large unions. Plus, I don't see any advantages in forcing input union branches to be anonymous, especially when the same is not true for output unions.

Anyway, that's my use case. I don't really consider it a "super mutation"; it's a simple mutation that just happens to use recursive input types. :)

@treybrisbane

This comment has been minimized.

Copy link

@treybrisbane treybrisbane commented Aug 5, 2018

@frikille Thanks for writing this up. :)

I definitely see why you went with the literal type. What was the rational behind going with a separate literal Foo declaration rather than just this:

input Foo {
  kind: "Foo"!
  # ...etc
}

???

@frikille

This comment has been minimized.

Copy link
Author

@frikille frikille commented Aug 9, 2018

@treybrisbane We were thinking about that and we think that supporting inline strings would be a nice syntax sugar but at the end, it would be parsed to a new type, so because of clarity we added the literal type (also I kinda prefer to declare everything properly)

@IvanGoncharov I'm not that familiar with Java any more, but it is a good point, and as far as I know, it doesn't have support for discriminated unions.

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Aug 9, 2018

I'm not that familiar with Java any more, but it is a good point, and as far as I know, it doesn't have support for discriminated unions.

Mobile is a pretty critical platform for GraphQL, so it would be great to know how it would be affected. So would be great if someone knowledgeable can answer my previous question:

how proposed input unions affect code generation especially for strong type languages, e.g. Java. I suspect that such languages don't support unions based on field value as the discriminator. It especially important for mobile clients.
Can someone check how it will affect apollo-ios and apollo-android?

@sorenbs

This comment has been minimized.

Copy link

@sorenbs sorenbs commented Aug 13, 2018

I am very happy that progress is being made on union input types 🎉

However, if I understand the proposal correctly, then it will not address the use cases we have in mind for the Prisma API.

Here is a spec for a new feature we will be implementing soon. I have written up how we would like to shape the API if union input types would support it. I have also described why the proposed solution with a discriminating field does not work for us: prisma/prisma#1349

I'm curious to hear if others are seeing use cases similar to us, or if we are special for implementing a very generic API?

Let me know if more details are needed.

@dendrochronology

This comment has been minimized.

Copy link

@dendrochronology dendrochronology commented Aug 13, 2018

@sorenbs would you mind explaining your reasoning a bit more, or perhaps provide some basic query examples, please? Perhaps I'm misunderstanding your atomic actions proposal, but it seems much broader than what's being discussed here.

I think the core issue this RFC addresses is to provide a simple, declarative way to allow for multiple input types on query and mutation fields. That's a severe weakness of GraphQL when it comes to data modeling.

A lot of my recent work has been porting data from legacy relational CMS's into modern GraphQL environments, and this RFC would solve one of the biggest headaches I've had. I'd rather see the community focus on feature-lean solutions to the biggest gaps first (i.e., things without good workarounds), and focus on narrower issues in later iterations.

@sorenbs

This comment has been minimized.

Copy link

@sorenbs sorenbs commented Aug 13, 2018

@dendrochronology - sorry for just linking to a giant feature spec, thats certainly not the best way to illustrate my point :-)

We have evolved our generic GraphQL API over the last 3 years. At 4-5 times throughout this process has it been brought up that some form of union input type could simplify the API design. Most of these cases boils down to supporting either a specific scalar type or a specific complex type:

given the types

type Complex {
  begin: DateTime
  end: DateTime
}

union scalarOrComplex = Int | Complex

type mutation {
  createTiming(duration: scalarOrComplex): Int
}

Create a timing providing duration in milliseconds or as a complex structure containing start and end DateTime:

mutation {
  simple: createTiming(duration: 120)
  complex: createTiming(duration: {begin: "2017", end: "2018"})
}

For us it is paramount that we maintain a simple API.

Did this example make it clear what we are looking to achieve?

It might also be worth noting that we have found it valuable on many occasions that A single element is treated the same as an array with a single element. This flexibility has made it easier to evolve our API in many cases, and I think we can achieve something similar for union input types if we choose a system that solves for that.

From this perspective, we would prefer the Tagged union by @leebyron and @IvanGoncharov described above. Keep in mind that I have not thought through the implications for tooling and strongly typed languages.

@dendrochronology

This comment has been minimized.

Copy link

@dendrochronology dendrochronology commented Aug 14, 2018

Thanks, @sorenbs, those are great examples. I do like the simplicity, and agree that robust APIs should handle singles vs. arrays elegantly, a 'la Postel's Law.

Back to @IvanGoncharov's request for a use case-driven discussion, I think that the union type implementation needs to go a bit further than generic APIs. A lot of my work is content and UX-based, where a client application pushes/pulls lots of heterogeneous objects from a back end, and displays them to a user for viewing/interaction.

For example, think of a basic drag-and-drop CMS interface, where a user can compose multiple blocks, with some levels of nesting and validation rules (e.g., block type X can have children of types A and B, but not C, etc.). With the current state of GraphQL, you need an awful lot of client-side logic to pre/post process output, because schemas have many fields with no baked-in validation or guarantees of meeting a required contract for available data or metadata fields (e.g., all blocks should have at least a type, label, color, x and y coordinates, ordinal number, etc.). I can mockup something like that in React easily, but building a GraphQL API to power it quickly gets messy.

I suppose my key takeaway is: I find that the current GraphQL spec makes it difficult to model a lot of real-world data situations without excessive verbosity and generous additional processing. I would love to be wrong, and have someone point out what I'm missing, but that hasn't happened yet 🤔

@jturkel

This comment has been minimized.

Copy link

@jturkel jturkel commented Aug 14, 2018

@IvanGoncharov - The proposed input union functionality for strongly typed clients should behave much the same way as "output"
unions. For Java, which doesn't support unions, this can be modeled via an interface representing the union that each member type implements. Simplified code for earlier createMedia mutation would look something like:

public interface AddMediaBlock { }

public class AddPostInput implements AddMediaBlock { }

public class AddImageInput implements AddMediaBlock { }

public class CreateMediaMutation {
  public CreateMediaMutation(AddMediaBlock[] content) { }
}

// Sample code that instantiates the appropriate union member types
AddMediaBlock[] content = new AddMediaBlock[]{new AddPostInput(...), new AddPostInput(...), new AddImageInput(...)};
client.mutate(new CreateMediaMutation(content));

Presumably Swift uses a similar strategy with protocols (the moral equivalent of Java interfaces).

@jturkel

This comment has been minimized.

Copy link

@jturkel jturkel commented Sep 10, 2018

@IvanGoncharov - What's your opinion on how to effectively move input unions forward? Seems like this issue has a pretty good catalog of use cases now and it's a matter of hammering out the technical details of input unions. It would be great to get a champion from the GraphQL WG to help guide this through the spec process. Is this something you have the time and desire to take on since you've already been pretty involved? I'm happy to help out in anyway I can (and I suspect others are too) but I'd like to make sure anything I do is actually helpful :)

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Sep 12, 2018

What's your opinion on how to effectively move input unions forward? Seems like this issue has a pretty good catalog of use cases now and it's a matter of hammering out the technical details of input unions.

@jturkel Agree and thanks for pinging 👍

It would be great to get a champion from the GraphQL WG

Working group don't have fixed seats and if you want to raise or discuss some issue just add it to the agenda.

It would be great to get a champion from the GraphQL WG to help guide this through the spec process. Is this something you have the time and desire to take on since you've already been pretty involved?

I can help with the spec and graphql-js PRs but before that, we need to formulate a more concrete proposal. I thought about it for last couple months but I still see a number of blind spots.

So as a next step I think we should find some points that we all agree on.
On weekends, I will try to summarize this discussion and my own thoughts in something that we could discuss.

@jturkel

This comment has been minimized.

Copy link

@jturkel jturkel commented Sep 13, 2018

Great to hear you're up for helping out with this @IvanGoncharov! Figuring out where folks agree/disagree sounds like a perfect next step to me. Feel free to ping me on the GraphQL Slack if there's anything I can do to help.

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Sep 17, 2018

@jturkel Update: Have an urgent work so I wouldn't be able to compile something until next weekends.

@dmitrif

This comment has been minimized.

Copy link

@dmitrif dmitrif commented Oct 17, 2018

Any updates on this? We are facing a similar need, unless there's a better way one may suggest:

input LocationQueryIdOptions {
  id: Int!
}

input LocationQueryCoordsOptions {
  latitude: Int!
  longitude: Int!
}

union LocationOptions = LocationQueryIdOptions | LocationQueryCoordsOptions

type Query {
  location(options: LocationOptions): Location
}
@amccloud

This comment has been minimized.

Copy link

@amccloud amccloud commented Oct 19, 2018

@dmitrif 👍 that's pretty much exactly what I tried before finding this issue:

input PostDescriptor {
  # ...
}

input ImageDescriptor {
  # ...
}

union ObjectDescriptor = PostDescriptor | ImageDescriptor;

type Mutation {
  Comment(objectDescriptor: ObjectDescriptor!, comment: CommentInput): CommentMutations
}
@gustavopch

This comment has been minimized.

Copy link

@gustavopch gustavopch commented May 3, 2019

@mplis OK, I agree there's some awkwardness on it. But if I understand the proposal correctly, it also adds an extra property kind. That's as awkward (if not more) than asking the index (or position, you name it). I say that because that kind is purely implementation detail while index can still be considered actual data. Also should clients assume that lists are always ordered lists? Isn't that an implementation detail as well? I mean, the list may be stored in the server as a hashmap keyed by ID or something. Who knows? I'm not sure the client should assume that the order will be preserved.

@AlessandroFerrariFood IMO it would be trivial to add a check like if (x && y) throw new Error('Both x and y were passed. Expected only one of them.').

@frankdugan3

This comment has been minimized.

Copy link

@frankdugan3 frankdugan3 commented May 3, 2019

@gustavopch The biggest issue with your solution from my perspective is that it defers something that could be statically validated to something validated at runtime server-side, and which must be implemented by the resolver which is not portable to other backend languages. I think increasing type safety within the spec itself is worthwhile.

@paean99

This comment has been minimized.

Copy link

@paean99 paean99 commented May 3, 2019

That solution and many others are practical. But don't they miss the point of graphql as a typed query language, since these solution are implemented in the resolvers?

edit: Sorry i am just repeating @frankdugan3 comment...

@gustavopch

This comment has been minimized.

Copy link

@gustavopch gustavopch commented May 3, 2019

@frankdugan3 @paean99 I think you are right.

@mike-marcacci

This comment has been minimized.

Copy link

@mike-marcacci mike-marcacci commented May 3, 2019

To chime in here again, I don’t think there is anything that is impossible with the current spec, since tagged unions are totally possible; I think the real goal here is better ergonomics and making invalid states unrepresentable. For example, if I want to accept a union type as an input argument, it’s quite trivial to use two named arguments with different types. Similarly, a heterogeneous list can simply be a list of wrapper types where each concrete type is relegated to its own field (this is the tagged union approach talked about by @leebyron).

So the context of discussion here must be about making this feel less awkward, or making stronger type guarantees for tooling. The latter comes into play when we consider that the schema has no way of describing how only one of the fields can contain content at a time. There is a bit of a moving line in this argument, as it’s outside the goals of GraphQL to represent all validation logic in the schema (for example, username character restrictions are clearly outside the scope of concerns).

From an ergonomic standpoint I am not totally convinced that the trade-offs in this proposal make for a cleaner API in general; they just make it different and I have yet to wrap my head around the consequences.

Story time: When I started writing Golang, I was really frustrated by the lack of genetics and overloading, and it felt dirty to write large numbers of very similar functions and structs. My experience with other languages set me up to fight go’s type system. But once I wrapped my head around and embraced the go idioms, they no longer seemed dirty or hacky, and the language worked quite well. I do feel that maybe some of the advantages in this proposal are simply changes to the mental model, which is not necessarily broken in the first place.

I don’t have super strong opinions either way, but wanted to express my concerns nonetheless.

@paean99

This comment has been minimized.

Copy link

@paean99 paean99 commented May 3, 2019

My personal opinion is similar to @mike-marcacci.
That is why i think this thread is so good, since it shows so many valid arguments for both sides.

There is a bit of a moving line in this argument, as it’s outside the goals of GraphQL to represent all validation logic in the schema (for example, username character restrictions are clearly outside the scope of concerns).

As an aside i would just like to point that graphql already has that functionality with Scalar types, if one so desire. It is just a question of defining our types a little more precisely.

@gustavopch

This comment has been minimized.

Copy link

@gustavopch gustavopch commented May 4, 2019

@frankdugan3 @paean99 I'm watching some talks in YouTube about GraphQL schema design. I think I'm seeing a consensus that polymorphism is not a good thing.

So instead of a single createMedia mutation that would accept different forms of inputs as the original post wants, the solution would be to have specific mutations for each kind of input: createPost and createImage.

Even if the client UI would present both things as a single form to the user, I think it makes sense to treat them as different things on the realm of GraphQL.

@bgentry

This comment has been minimized.

Copy link

@bgentry bgentry commented May 4, 2019

@gustavopch while there are certain simple scenarios for which separate GraphQL endpoints make sense, there are others where that would result in a crazy amount of API bloat. How would you address the IFTTT use case I commented on previously? Multiple separate mutations for each of the hundreds of supported services?

@gustavopch

This comment has been minimized.

Copy link

@gustavopch gustavopch commented May 4, 2019

@gustavopch while there are certain simple scenarios for which separate GraphQL endpoints make sense, there are others where that would result in a crazy amount of API bloat. How would you address the IFTTT use case I commented on previously? Multiple separate mutations for each of the hundreds of supported services?

Could you post an example of how your schema would be if GraphQL had input union types?

@frankdugan3

This comment has been minimized.

Copy link

@frankdugan3 frankdugan3 commented May 4, 2019

@gustavopch There is already polymorphism in GraphQL, the issue here is the incongruity between types and input types. If the consensus is that polymorphism is bad, then we should get rid of Interface and Union. I don't see that happening... So since those constructs are here to stay, I think it makes a lot of sense to simplify mutations by adding similar polymorphism for input types.

@acao

This comment has been minimized.

Copy link

@acao acao commented May 7, 2019

If folks want to see a demo of inputUnion similar to what I showed in the WG meeting last week, here ya go:

https://github.com/acao/apollo-graphql-input-union-example

I'll set up a live demo soon, with a working schema explorer

@alamothe

This comment has been minimized.

Copy link

@alamothe alamothe commented May 9, 2019

Amazing work! :-) my vote goes for __inputname proposal. It's the cleanest approach (as in, transparent to the user) and consistent with how output types work.

@akomm

This comment has been minimized.

Copy link

@akomm akomm commented Jun 7, 2019

@mike-marcacci #488 (comment)

To add to @treybrisbane's example, here on the mutation & input side:

input TestAInput {
  valueA: String!
  valueB: String!
}

input TestBInput {
  valueA: String!
  valueB: String!
}

type Mutation {
  testA(input: TestAInput!): TestAPayload!
  testB(input: TestBInput!): TestBPayload!
}

Try run this (TestBInput passed to testA which requires TestAInput):

# Variable type 'TestBInput!' doesn't match expected type 'TestAInput!'
mutation IsNotStructural($input: TestBInput!) {
    testA(input: $input) {
        ... on TestASuccessPayload {
            # ...
        }
        ... on TestAFailurePayload {
            # ...
        }
    }
}

Additionally, consider this is also an error:

# Unknown field "valueC" on input type "TestAInput". Did you mean "valueA", or "valueB"?
mutation IsNotStructural {
    testA(input: {valueA: "a", valueB: "b", valueC: "c"}) {
        ... on TestASuccessPayload {
            # ...
        }
        ... on TestAFailurePayload {
            # ...
        }
    }
}

For me graphql looks very nominal.

@mike-marcacci Your example has nothing to do with structural typing and is just a lucky case in which your new types have a common shape and thereforce you can swap it with a common interface.

I am strongly for input unions, if needed with __inputname. I have cases where input variants are mutually exclusive. I have to create different Payload/Input types and mutation fields to represent all those possible variants. Even if you write some abstraction over the type system which generates all those types for you, the whole system is polluted with tons of similar named types.

I am a total fan of @leebyron's viewpoint about solving problem with least possible concepts. This Is what I love about many FB's os projects. They seem to follow this concept. However I think this is something that is dire needed for the many reasons iterated in this discussion.

@benjie

This comment has been minimized.

Copy link
Member

@benjie benjie commented Jun 7, 2019

@akomm I'd be interested to know if #586 solves your requirements? (Please discuss in that thread.)

@akomm

This comment has been minimized.

Copy link

@akomm akomm commented Jun 7, 2019

@benjie I was initially rather with the inputUnion approach following this discussion. But after reading your post and thinking about it I found issues with inputUnion. Additionally, while you focus on the input topic there, in my opinion @oneField can even be interesting in context of output types. I am currently using unions for mutation success and failure outcomes (failures representing user errors, not exception, etc.). I have to utilize __typename for this and this does not feel as a good solution to me. With @oneField I could return to an older approach I had, where I could simply have success and failure fields which are never non-null at once. I did not like it because the mutual exclusiveness was not guaranteed by the type system.

@benjie

This comment has been minimized.

Copy link
Member

@benjie benjie commented Jun 7, 2019

@akomm Indeed, enabling it to cater to certain output situations was something I kept in mind whilst writing the PR. I don't want to start that discussion yet though, I feel it would muddy the waters (and the input use case is a lot more compelling IMO). I'm glad you spotted that too though!

@akomm

This comment has been minimized.

Copy link

@akomm akomm commented Jun 7, 2019

@benjie

What we actually have then with your proposal is similar to algebraic enumeration, like union, however union semantics have the issue that you loose the type information at runtime when you receive data, making the need of an __typename property to transport it.

WIth the tagged union the variants of the enumeration are transported with the data and accessible by client (or the other way by backend when we speak about input), therefore no __typename or __inputname is needed.

However currently no new keyword/type is introduced, simply because enum already exists non algebraic and changing this would probably be a BC and involve more work and more time to introduce a well working solution, than the annotation. And I think this semantic thing is something that throws some people off.

I mean the @oneField might also be potentially a @enum, however I see there might be confusion with the enum that already exists.

@shadrech

This comment has been minimized.

Copy link

@shadrech shadrech commented Jun 19, 2019

A lot of discussion on this subject. Any code merged?

@alamothe

This comment has been minimized.

Copy link

@alamothe alamothe commented Jul 17, 2019

One obvious way to do this is pack everything inside a JSON, and pass it as a string parameter. The JSON should have __inputname where appropriate. Then on the server just parse and process it as if it was a union. If using TypeScript, it's easy to define proper types and get type checking.

The drawback is that the type is not visible in the GraphQL schema.

If you're desperate enough... it works 🙂

@nuttycom

This comment has been minimized.

Copy link

@nuttycom nuttycom commented Aug 14, 2019

There have been a couple of comments along the way suggesting that tagged union types are not supported in typed languages like Java. This is incorrect - tagged unions (sum types) are represented in these languages using the Visitor pattern (the corrected version thereof: http://logji.blogspot.com/2012/02/correcting-visitor-pattern.html)

For those of us used to working in languages like Haskell, Scala, or OCaml which have proper support for algebraic data types, it seems obvious that sum types should be just as faithfully represented for input as they are for output.

@Vadorequest

This comment has been minimized.

Copy link

@Vadorequest Vadorequest commented Oct 11, 2019

This RFC was opened 14 months ago, and this limitation has been limiting GraphQL users for years now. Something should have been done by now, type polymorphism is a basic feature in many modelling tools and it's limiting all tools built using GraphQL.

At the very least, could we have a "status summary" in the main post, so that it's easier to follow this thread? As some other people pointed out, there has been extensive discussion in the thread and it's hard to follow.

In this case (that seems complex), let's not forget that "best" is often worse than "good enough".

Something needs to happen. This has been/is being implemented by GraphQL developers over and over.
Prisma is currently working on their own RFC for instance because the official RFC feels stuck.

@dendrochronology

This comment has been minimized.

Copy link

@dendrochronology dendrochronology commented Oct 11, 2019

It'll actually be 4 years next week, if you count graphql/graphql-js#207 😓

I agree with @Vadorequest . I appreciate the thoroughness of the discussion here, but I fear that perfect has become the enemy of done.

This seems to be a serious weakness when it comes to modeling real-world, non-trivially complex data structures, and making reasonable GraphQL APIs for accepting said structures. As such, we resort to workarounds like: giant JSON blobs, ugly/hack-ish/insanely verbose GQL schemas, patching/customizing core GraphQL tools, or going around GraphQL entirely and side-loading data via REST, etc. My team has done all of those things at some point, and none of them are much fun.

I realize that there are tradeoffs with all the proposed solutions, but what we're talking about is somewhat advanced usage, so I don't mind doing a bit more work on the backend when it comes to coding resolvers and mutations, like handling a discriminant field, or needing to understand a bit more about possible shapes of data. That may not be perfectly clean, but neither is the real world 😛

I'd love to see the spec team make an executive call on how to handle this, so folks in the community can plan accordingly.

@benjie

This comment has been minimized.

Copy link
Member

@benjie benjie commented Oct 15, 2019

This topic is actively discussed by the working group; here's notes from some recent WGs:

We agree that "best" is often worse than "good enough" (aka the perfect is the enemy of the good). We are actively working on the Input Union RFC document to help us analyse the various goals and trade-offs that are at work; the document exists here: https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md and has a few open PRs to it right now, including:

We all feel this pain, which is why we're actively working to solve this particular issue. There is no perfect solution (that we have found) so we are all working towards being able to pick a solution that is acceptable to everyone, even if it's not perfect for all use cases.

You can get involved by contributing to the input union RFC document with your use-cases, aims, and the trade-offs that you would like to avoid. You can also become involved with the working group to help move the conversation on.

@binaryseed, who is leading this charge, has proposed in #627 that this and many other of the input union/input interface/input polymorphism issues and pull requests are closed in favour of more focussed discussion around the RFC document itself.

You may also be interested in reading the "Guiding Principles" of the GraphQL spec as that will help to understand how we are comparing the various options.

@IvanGoncharov

This comment has been minimized.

Copy link
Member

@IvanGoncharov IvanGoncharov commented Nov 5, 2019

We have begun building a single RFC document for this feature, please see:
https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md

The goal of this document is to consolidate all related ideas to help move the proposal forward, so we decided to close all overlapping issues including this one.
Please see discussion in #627

If you want to add something to the discussion please feel free to submit PR against the RFC document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.