RFC: inputUnion type

[tgriesser]

Related graphql-js PR: graphql/graphql-js#1196

This is an RFC for a new type: inputUnion.

An inputUnion is a union of one or more input types. It may be used in any location where an input is currently valid. When fulfilling an inputUnion an additional field __inputname must be specified in the map/object fulfilling the input, where the value of __inputname is the name of a single member of the inputUnion being fulfilled.

Example:

input PostInput {
  title: String!
  body: String!
}
input ImageInput {
  photo: String!
  caption: String
}

inputUnion MediaBlock = PostInput | ImageInput

type Mutation {
   addContent(content: [MediaBlock]!): Post   
}

mutation AddContent($content: [MediaBlock]!) {
   addContent(content: $content) {
      id
   }
}

Valid $content value:

[
  {__inputname: "PostInput", title: "Hello", content: "World"},
  {__inputname: "ImageInput", photo: "http://graphql.org/img/logo.svg", caption: "Logo"}
]

Invalid Value Examples:

{__inputname: "PostInput", title: "Invalid, missing 'content'"}

{title: "Invalid, missing __inputname", content: "World"}

{
  __inputname: "PostInput", 
  title: "Invalid, photo is not defined on PostInput", 
  content: "World", 
  photo: "http://graphql.org/img/logo.svg"
}

Checklist:

Are we solving a real problem.

Yes. Many of these problems or use cases are laid out in graphql/graphql-js#207 but to summarize:

When creating input objects, both in mutations and queries you face a tradeoff when creating complex input structs, with one of two options:

1. Enforce a well typed structure of the input via required fields !. Create multiple endpoints (mutation or query) utilizing these various strict, special case input types.
2. Loosen the input type requirements and rely on runtime/server-side validation to determine the intended uses.

This solution aims to offer a third path, where more complex combinations of strict input combinations may be utilized, while still keeping the input types fulfillment unambiguous via the __inputname field requirement.

Does this enable new use cases.

Yes. Many of the use cases are detailed in graphql/graphql-js#207. I think the biggest thing this unlocks is the list of heterogeneous inputs, which can be used to define an ordered set of instructions. This also reduces the need for many individual mutations while being able to maintain strictly typed inputs. In my experience tools like apollo-codegen and graphql-code-generator have proven invaluable in creating Flow/TypeScript definitions for validating queries. This change will work well in combination with those tools, making complex input semantics simpler to statically check.

How common is this use case.

Very common. This is the most commented issue in graphql-js, and I personally have run into the tradeoff of creating many highly restrictive mutations vs loosening them up and creating an ad-hoc server implementation. This sort of concept feels like the missing corollary to the expressiveness of the graphql Query execution (single "smart" entry point rather than many ad-hoc endpoints).

Can we enable it without a change to GraphQL.

No, at least not without pushing any type-checking semantics to the execution layer.

Additional thoughts

What about interfaces? There are several comments in related tickets expressing a desire for interfaces in addition to input unions. While it sounds nice for symmetry with querying, I don't see these as being useful or necessary in practice at the input layer. Interfaces are most useful when you wish to query for a generic partial representation of a more specific type. This same requirement does not exist for inputs and it is my opinion that inputInterface would not add enough additional value to justify its addition.

Open questions:

• Is the use of __inputname a valid option based on spec (__ is reserved for introspection, not sure if we can mirror this for execution)
• Does __inputname make sense as the name for this?

[treybrisbane]

Thanks for taking the time to put this PR (and the associated graphql-js PR) together! :)

A couple of things on this:

1. What was the reason you decided to add an explicit inputUnion keyword? Have you considered simply extending the semantics of the existing union keyword to work for both input and output types while preventing any mixing of the two?
2. What alternatives approaches have you considered for discriminating the input? While __inputname is effective, it's a bit on the ugly side, and as you already pointed out is reserved for introspection...

Given that when querying a union type we must discriminate using specific syntax (i.e. a conditional fragment), it would make sense if we could somehow employ a syntactic approach to discriminating inputs. One example is:

mutation {
   addContent(content: [{ title: "Foo", body: "Bar" }] as PostInput) {
      id
   }
}

This would give you the ability to discriminate without including a magic field (GraphQL could pass on the "as X" type name to resolvers via the info argument), however it would mean that for named mutations, you'd need to define one per union member (which may or may not be a bad thing). E.g.

mutation AddPostContent($content: [PostInput]!) {
   addContent(content: $content as PostInput) {
      id
   }
}

mutation AddImageContent($content: [ImageInput]!) {
   addContent(content: $content as ImageInput) {
      id
   }
}

The above is just an example, but the takeaway is that it would make sense to discriminate inputs via some syntactic construct if possible, given that that's how we discriminate outputs.

Alternatively, if we can't (or don't want to) do things with syntax, then we should probably consider a way to make the name of the discriminant field configurable.

Thoughts?

[tgriesser]

What was the reason you decided to add an explicit inputUnion keyword? Have you considered simply extending the semantics of the existing union keyword to work for both input and output types while preventing any mixing of the two?

Yes. Most of the comments in the related PR's and tickets mentioned wanting to re-use union for this purpose. However when I began to implement it that way, it ended up feeling sloppy / difficult to implement for all possible cases. Using union for both purposes made it difficult to have strong guarantees about the types internally in graphql-js. I also realized mixing the two could inhibit future use-cases that are specific to querying vs. inputs.

What alternatives approaches have you considered for discriminating the input? While __inputname is effective, it's a bit on the ugly side, and as you already pointed out is reserved for introspection...

I don't know that it's necessarily reserved for introspection, that was sort of an open question. In querying it is reserved for introspection but I'd imagine it'd be reasonable to assume __ as a pseudo-reserved prefix for the language in general, and could be repurposed for this case.

I wouldn't focus on the ugliness, as queries utilizing input unions are unlikely to be written by hand in the query string.

The above is just an example, but the takeaway is that it would make sense to discriminate inputs via some syntactic construct if possible, given that that's how we discriminate outputs.

Outputs are generally discriminated at runtime via __typename. This is the inverse of that, where we're tagging the input.

type -> union -> __typename :: input -> inputUnion -> __inputname

Also, in your example above, you lose the ability to have a list of heterogeneous input types, which is one of the big wins of this addition.

Alternatively, if we can't (or don't want to) do things with syntax, then we should probably consider a way to make the name of the discriminant field configurable.

I would disagree, I think this would be better standardized than configurable.

[treybrisbane]

Thanks for the response! :)

However when I began to implement it that way, it ended up feeling sloppy / difficult to implement for all possible cases. Using union for both purposes made it difficult to have strong guarantees about the types internally in graphql-js. I also realized mixing the two could inhibit future use-cases that are specific to querying vs. inputs.

Sounds fair enough to me. :)

Yeah, the more I think about it, it seems like it may have been better for input to be more of a "qualifier" (i.e. input type Foo { ... } and input union Bar = ... rather than just input Foo { ... } and inputUnion Bar = ...), but alas the ship has well and truly sailed on that.

Also, in your example above, you lose the ability to have a list of heterogeneous input types, which is one of the big wins of this addition.

Ahh, you're absolutely correct. I admit I had not considered that case. Thanks for pointing that out. :)

Outputs are generally discriminated at runtime via __typename. This is the inverse of that, where we're tagging the input.

I would disagree, I think this would be better standardized than configurable.

You're correct in that __typename can be used for this purpose, however in practise I have always included a dedicated type/kind field that is used as a discriminant for my unions. The reason being that I prefer that information to be captured explicitly in the domain model, rather than relying on an implementation-specific API feature. This may not be the "default" or "preferred" way of discriminating in queries, but the point is that GraphQL is flexible enough to support this. I think that we should have that same flexibility with input unions.

Since we're already looking at adding new syntax (i.e. inputUnion), perhaps we could look at extending that slightly with a way of explicitly specifying the discriminant field? E.g.

inputUnion MediaBlock by fieldNameHere = PostInput | ImageInput

Or

inputUnion MediaBlock(discriminant: fieldNameHere) = PostInput | ImageInput

You could potentially even use the directive syntax here (defaulting to __inputname if not specified):

inputUnion MediaBlock @discriminate(by: fieldNameHere) = PostInput | ImageInput

Again, the syntax is open to change here, but I think being able to somehow specify this in the schema is important.

I can understand where you're coming from with regards to standardisation over configuration, but we won't have anything to standardise if we can avoid using magic field names like __inputname in the first place. :)

[Yogu]

it seems like it may have been better for input to be more of a "qualifier" (i.e. input type Foo { ... } and input union Bar = ... rather than just input Foo { ... } and inputUnion Bar = ... ), but alas the ship has well and truly sailed on that.

Would it not be possible to still use input union here? I like that much better than a keyword in camelCase.

We would need a little lookahead in the parser to see if union is a keyword or the name of an input object, but I guess that would work by checking if the next token is {.

[tgriesser]

Would it not be possible to still use input union here? I like that much better than a keyword in camelCase.

@Yogu Sounds reasonable! I don't really have a strong opinion here.

The reason being that I prefer that information to be captured explicitly in the domain model, rather than relying on an implementation-specific API feature.

@Tbrisbane I don't believe __typename is implementation-specific, it's standardized in the spec and is the way tools like Apollo & Relay determine object types when querying. Standardizing makes it simpler to build predictable tooling around these core concepts.

[theorygeek]

should say:

the concrete Input Object type

[leebyron]

Thanks for drafting this up, Tim! This is a really interesting approach to this problem.

In seeking a comparison point to draw parallels, pros and cons I think this tagged-union pattern is closest to your initial example:

input PostInput {
  title: String!
  body: String!
}
input ImageInput {
  photo: String!
  caption: String
}

# Note: this is the only change from the example type system.
# A server may wish to add dynamic enforcement that one field is provided
input MediaBlock {
  postInput: PostInput
  imageInput: ImageInput
}

type Mutation {
   addContent(content: [MediaBlock]!): Post   
}

mutation AddContent($content: [MediaBlock]!) {
   addContent(content: $content) {
      id
   }
}

Which would have the corresponding valid $content input of:

[
  // Note: no need for a special reserved string key, but contains semantically 
  // similar content as example.
  {postInput: {title: "Hello", content: "World"}},
  {imageInput: {photo: "http://graphql.org/img/logo.svg", caption: "Logo"}}
]

---

In looking at the above, I'd like to challenge your answers to "Does this enable new use cases?" and "Can we enable it without a change to GraphQL?"

Can you help clarify what new possibilities or use cases are enabled by inputUnion compared to the tagged-union pattern? You've clearly put a lot of thought into this, so I'm looking forward to your more reasoned analysis, but my initial impression is that this is mostly a trade between strictness (tagged-unions don't statically enforce exactly one used field) and complexity (being that this proposal adds to both the type system and the request protocol).

[tgriesser]

Thanks for the feedback, Lee!

but my initial impression is that this is mostly a trade between strictness (tagged-unions don't statically enforce exactly one used field) and complexity (being that this proposal adds to both the type system and the request protocol).

That is exactly the trade!

However I view the complexity from a different perspective. Implementing these sorts of dynamic checks ad-hoc during execution (as you mention would be warranted in the comment for MediaBlock), while also inhibiting the possibility for tooling to generate accurate static typings or valid API documentation from inputs is more complex than the alternative of including this (or similar) concept in the core type system.

Should { postInput: { ... }, imageInput: { ... } } be considered a valid input? If no, why must that responsibility fall to the server execution rather than the schema specification and input validation?

The example you provide requires wrapping each input value in an additional input object. When creating input shapes more complex than this contrived example (nested and otherwise), it feels like an antipattern to need to emulate unions in this fashion, for lack of a better built-in alternative.

To think of a parallel on the query side: If the union type didn't exist, it'd certainly be possible to just utilize a wrapping type with a bunch of nullable fields for each of the potential members. Then it'd be up to the resolving implementation to enforce only one is fulfilled, and the consumer to figure out based on nulls which one the server means to represent:

query {
   contentBlocks {
       postContent {
         title
         content
       }
       imageContent {
         photo
         caption
       }
   }
}

While it initially appears more complex, in practice it's much simpler for tooling, type generation, and end user code to utilize union or interface and __typename:

query {
   contentBlocks {
       __typename
       ... on Post {
         title
         content
       }
       ... on Image {
         photo
         caption
       }
   }
}

[IvanGoncharov]

I like the idea of allowing input objects to be a more flexible but still be able to fully validate it.
However, I see a few critical issues in current proposal:

1. __inputname is bad UX for front-end devs and makes a very steep learning curve.
2. It forces me to have __inputname inside my source code even if I use query variables. For example:

    mutation AddContent($content: [MediaBlock]!) {
      addContent(content: $content) {
        id
      }
   }

   I would need to call it like AddContent([{ __inputname: "...", }]) in my source code. Moreover, if I need to write/generate typings they also would be poisoned by __inputname.
   And if compared to __typename inside response, I can't alias it to something like contentType. I don't think forcing clients to have GraphQL keywords outside of query string is a good idea.
3. It's breaking change for almost all client tools that work with GraphQL introspection results.

---

I'd like to propose an alternative solution:

input MediaBlock = { post: PostInput! } | { image: ImageInput! }

So the idea is to allow define input object as disjointed sets of fields with the following requirements:

• You can't have field with the same name in multiple sets
• Every set should have at least one non-null (required) field

It not only adds type safety to tagged-union pattern suggested by @leebyron above but also allows to eliminate it if your sets of fields don't have conflicting fields:

input Date = { date: String! } | {
  day: Int!
  month: Int!
  year: Int!
}

We can make it a non-breaking change for client tools by extending __Type:

type __Type {
  kind: __TypeKind!
  # ...

  # INPUT_OBJECT only
  inputFields: [__InputValue!] @deprecated(reason: "Use `inputFieldSets` instead")
  # INPUT_OBJECT only
  inputFieldSets: [[__InputValue!]!]
}

Old clients will continue query for inputFields so for them MediaBlock will efectively look like this:

input MediaBlock {
  postInput: PostInput
  imageInput: ImageInput
}

New clients will use inputFieldSets to provide client-side validation or better documentation.

@tgriesser What do you think about this proposal?
If you think it's out of scope for this issue I can create a separate one.

[tgriesser]

__inputname is bad UX for front-end devs and makes a very steep learning curve.

I would respectfully disagree on both of these assertions.

Being explicit about which input type member we mean seems like it could only yield better DX, particularly when combined with TypeScript & Flow generated types. Thinking of autocomplete for graphiql, once the __inputname is provided (valid names would already be known from introspection), it's straightforward to autocomplete the rest of the input member fields.

I also wouldn't say the learning curve is steep, to the extent that __inputname is only necessary if using a inputUnion (or input union) type. Folks could write query/mutation arguments using just input and avoid needing to learn this topic altogether, at least until they encounter something which doesn't fit well with input limitations.

Further, I would say the learning curve would be eased by helpful errors are provided when __inputname is omitted.

It forces me to have __inputname inside my source code even if I use query variables. For example:

Again, only if you utilize this new type, which is not required to have a working GraphQL query.

If the __inputname is the apprehension here, I could imagine wrapping with a helper function:

const PostInput = (a) => ({ ...a, __inputname: 'PostInput' })
const ImageInput = (a) => ({ ...a, __inputname: 'ImageInput' })

AddContent([
  PostInput({ ... }),
  ImageInput({ ... })
])

I suspect most uses of this type would have some sort of abstraction built around this and hand-written mutations with __inputname would be kept to a minimum.

It's breaking change for almost all client tools that work with GraphQL introspection results.

I'm interested to hear more about this. As it's an additive change (new type), I can't imagine it would break too much existing tooling, at least no-more so than a potential change to the structure of input altogether.

You can't have field with the same name in multiple sets

That's actually one of the big motivations here, and I think would be a non-starter.

Trying to consolidate:

mutation {
  updateAccountBasic(input: UpdateAccountBasicInput!): Account!
  updateAccountContact(input: UpdateAccountContactInput!): Account!
  updateAccountFromAdmin(input: UpdateAccountFromAdminInput!): Account!
  updateAccountPreferences(input: UpdateAccountPreferencesInput!): Account!
}

into:

inputUnion UpdateAccountInput = UpdateAccountBasicInput | UpdateAccountContactInput | UpdateAccountFromAdminInput | UpdateAccountPreferencesInput

mutation {
  updateAccount(input: UpdateAccountInput!): Account!
}

Where all of these inputs contain an id field, and perhaps some combination of the other fields.

Yes, we could do:

input UpdateAccountInput = {
  basic: UpdateAccountBasicInput
  contact: UpdateAccountContactInput
  fromAdmin: UpdateAccountFromAdminInput
  preferences: UpdateAccountPreferencesInput
}

mutation {
  updateAccount(input: UpdateAccountInput!): Account!
}

But now we've lost the type safety and intention of the API, added an additional input type, and added four additional "names" to keep track of (the keys of the input object). I guess it seems analogous to using the wrong data structure to express a value.

---

But with all that said, I'm not necessarily convinced on tagging the input with __inputname as the best final solution either, so I appreciate pushback/different ideas.

[acjay]

Given the suggestion that this problem can be worked around by using composition instead of subtyping, I'd pose the question: is union a mistake?

I think it's totally reasonable either way to choose a richer language for data modeling or sticking to a small set of core building blocks. But it seems strange to me to make different choices for input and output type modeling. Unless there's a distinction I'm missing.

[tgriesser]

I suppose another option is to indicate usage with a custom directive (is this valid syntax/how directives should be used?):

input UpdateAccountInput @inputUnion {
  UpdateAccountBasicInput: UpdateAccountBasicInput
  UpdateAccountContactInput: UpdateAccountContactInput
  UpdateAccountFromAdminInput: UpdateAccountFromAdminInput
  UpdateAccountPreferencesInput: UpdateAccountPreferencesInput
}

And then any type-gen utilities could key off the presence of the @inputUnion directive on the type to generate the definition as:

type UpdateAccountInput = 
 | { UpdateAccountBasicInput: UpdateAccountBasicInputType }
 | { UpdateAccountContactInput: UpdateAccountContactInputType }
 | { UpdateAccountFromAdminInput: UpdateAccountFromAdminInputType }
 | { UpdateAccountPreferencesInput: UpdateAccountPreferencesInputType };

Still feels like a suboptimal solution, but I think I'd take it over the alternate syntax for input

[IvanGoncharov]

@tgriesser Using directive is a great idea and definitely the safest path 👍
In your example, you are using type names as field names.
Is it required or I can rewrite your example into this one?

input UpdateAccountInput @inputUnion {
  basic: UpdateAccountBasicInput
  contact: UpdateAccountContactInput
  fromAdmin: UpdateAccountFromAdminInput
  preferences: UpdateAccountPreferencesInput
}

Bikeshedding: I like tagged-union term suggested earlier so can we change it to @taggedUnion?

[treybrisbane]

For what it's worth, I strongly disagree with any reliance on the so-called "tagged-union pattern", even if it's extended with directives. It's very easy for us to use contrived and simplistic examples of the tagged-union pattern to justify not adding first-class language support for input unions, but in practise, my experience has shown that this pattern does not scale.

As is the case with any language, we should definitely be conservative in regards to adding features to GraphQL. But we need to be careful that we don't "err" too hard on the conservative side and risk dismissing features that stand to solve an ongoing problem for GraphQL's users.

I see the current state of affairs as this:

• There is a valid need for this (Union input types #202, GraphQLUnionType as input type graphql-js#207, Any chance for a input union support?  graphql-rust/juniper#18, etc...)
• The best workaround so far (the "tagged-union pattern") obscures the true intent of the design, and adds a not-insignificant amount of complexity to both schema definitions and field resolution logic
  • This complexity multiplies with every instance of the pattern within a schema, making the cost too high for more than simple use-cases
• At a more academic level, we have an asymmetry in the language in that there is first-class support for output unions, but no equivalent for inputs

Given the above, I think we have just-cause for adding first-class language support for input unions. Or, to put it another way, I don't think that the value of input unions is low enough to justify continued reliance on patterns/workarounds in lieu of first-class language support, especially when said patterns don't scale.

[treybrisbane]

Regarding the proposals and discussion thus far, I see @tgriesser 's original proposal as having the following pros and cons:

• 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

Additionally, I see @IvanGoncharov 's alternative proposal as having the following pros and cons:

• 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

Finally, I see @tgriesser 's @inputUnion idea as having the following pros and cons:

• 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

Based on all this, it is clear to me that @tgriesser 's original proposal is by far the best option so far. So, if that proposal isn't good enough in it's current form, my question is this:

What do we need to change in the original proposal to make it acceptable?

[tgriesser]

In your example, you are using type names as field names.

@IvanGoncharov that was intentional, the goal being to eliminate needing yet another name to express something, by just duplicating the input type name. I would want to sidestep questions of:

• should the field name be the camel-cased form of the input type?
• can it be whatever?
• if not, how do we enforce that?

All things that a union type inherently avoids by just being a set of named types.

---

@Tbrisbane Well summarized

Adds complexity to the language in the form of a new directive

If I understand correctly, users can now define custom directives in the schema definition language. My idea there was if this were to fall to being implemented in userland, that's a possible solution without needing any language extension (pro and con).

I would still love to see something baked-in for all of the reasons you mentioned, but if there isn't a consensus I'd understand as well.

[treybrisbane]

Hey guys, has there been any further developments on this? I currently maintain two GraphQL APIs, and this continues to be a pain point for me.

In the interests of moving forward with this, can people concisely list out any specific things from @tgriesser's proposal that they think need either changing or further thought?

[treybrisbane]

For me, I have reduced that list to only a single minor item:

• Change the inputUnion camel-case keyword to be either just union or inputunion

We could argue in favour of union by pointing out that GraphQL deliberately separates input and output object types by using separate keywords (input and type, respectively). Contrast this to the scalar and enum keywords, which do not have separate input and output keywords. We could see the union keyword as being the same as scalar and enum in that it could be used for both inputs and outputs, with a stipulation added to its semantics which states that all members of a union MUST BE the same kind of type; either they MUST ALL BE inputs, or they MUST ALL BE outputs.

In a similar vein, we could argue in favour of inputunion by acknowledging that input unions actually add to the request protocol, so there is value in making this distinct with a separate keyword. Additionally, Tim has stated that there are implementation details that make a separate keyword more attractive.

Honestly, I'd be happy with either of those. I'm just not a fan of a camel-cased keyword, given that most languages seem to avoid this (e.g. instanceof, foreach, etc).

Other than that, I think Tim's proposal is sound, despite my earlier misgivings around __inputname, etc. :D

[serle]

This is the biggest issue that I have with my GraphQL API. I end up having to use unions on outgoing calls and JSON scalars on incoming calls (as a bad work-a-round). For me the biggest benefit of GraphQL is schema validation and the API contract that develops between client and server.

Not using polymorphic/union capability results in huge API bloat or really inefficient data structures where the front-end ends up having to manage multiple strongly typed data fields which are all set to null bar one.

I fully support @tgriesser approach and would prefer using lowercase union and __inputname keywords. The increase in syntax complexity is insignificant and actually makes GraphQL clearer and far richer IMHO.

I would also like to see the overlapping field restrictions on unions and input unions dropped.

Please, lets get this finalized soon!!!

[schickling]

@tgriesser maybe you (or someone else) would love to join for the next GraphQL working group meeting to discuss how to move forward with this proposal?

[serle]

Is there any timeframe for when we can expect input unions to make it into the draft specification and when the draft specification is expected to be finalized?

Would it be possible to create a roadmap file for this and other upcoming changes?

[jturkel]

It would be nice if changing an input field from an input object type to an input union type was backwards compatible since it's often hard to predict where polymorphism will be required in a data model. Given that, should we allow any input object (i.e. not just an input union member) to optionally include the __inputname attribute? The recommendation would be that client side tooling always include the __inputname argument to better support schema evolution even if it's only required for input unions.

[Yogu]

Or just allow to specify a default union option that is assumed by the server when __inputname is missing. Joel Turkel <notifications@github.com> schrieb am Do., 3. Mai 2018, 22:48:

…

It would be nice if changing an input field from an input object type to an input union type was backwards compatible since it's often hard to predict where polymorphism will be required in a data model. Given that, should we allow any input object (i.e. not just an input union member) to *optionally* include the __inputname attribute? The recommendation would be that client side tooling always include the __inputname argument to better support schema evolution even if it's only required for input unions. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#395 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAYKIj7tsKni2VsuPvVWDx7mN1YubODoks5tu20ggaJpZM4RbYsm> .

[nodkz]

Yesterday I compared ___typename with Generics in TypeScript/Flow. And ask why we should add additional __inputname, something like non-existent in TS/Flow GenericsInput? Why not keep __typename also for InputUnion.

Now, I absolutely agree with __inputname. A client cannot take query response and put its result to mutation – need transform Type name. Returned output object type will not be accepted by mutation (need provide input object type with another name). It cannot match the query side. So to make the requirement of such transformation more obvious __inputname is a good candidate with the current GraphQL implementation. 👍

---

Anyway, if GraphQL type system was closer to TypeScript/Flow/OtherTypedLang, and do not provide additional InputObjectType entity, instead uses regular ObjectTypes – then it will be much simpler to reuse existed object types as inputs. We perfectly use in TS/Flow the same type in functions for args and result; and if we need to differ them – just introduce a new type. But it's a theme for another discussion and touches much deeper parts of GraphQL design.

---

According to the current differ output/input types by GraphQL design – the __inputname is I right choice. If you don't like it, so firstly try to remove InputObjectType class in favor of ObjectType (but there are will be so many questions with runtime, eg. how to deal with args and resolve properties for input data).

[jodinathan]

Honestly, inputName, typeName, whatever you want, just get into a conclusion in this meeting and let this be implemented as soon as it can

[asprouse]

Yesterday there was discussion of creating a work draft in the repo similar to what was done for subscriptions. I think this would help unify all the proposed solutions. @tgriesser Your solution proposed here seems to be the closest to a consensus. @leebyron and others had concern about requiring a descriminator property (vs inference + ordered list of types), but having one RFC for everyone discuss would create a forum for this discussion. Does it make sense to create rfs/InputUnions.md in the context of this PR?

[jbblanchet]

I totally agree with @jodinathan that any solution would be appreciated.

@asprouse Could a compromise between discriminator property and a more organic solution be that the discriminator property could be configurable. It could be:

• a convention (for example the first property of the parent type or of each member of the union type)
• an attribute (@typename)
• new keyword (input PostInput { typename type: String!, title: String!, body: String! })
• new special character (input PostInput { type: String$, title: String!, body: String! })

That would combine the fast resolve of the discriminator with the adaptability of inference.

[binaryseed]

Since the Working Group meeting, I've been pondering @leebyron's challenge to see if we can design an inputunion without a discriminator field, and I'd like to toss out a solution to see if it might work.

Most of my thinking so far has been about the "runtime" behavior of the schema, but if we step back - I think there might be a "compile time" rule we can put in place that makes this a whole lot simpler. This would be a rule we enforce before a schema is even considered valid.

Schema Rule:

• Each type in the input union must have a unique set of required fields.

With this rule in place, I believe it might be possible to do structural type discrimination w/o relying on any of the previous band-aids (__typename, etc).

With this simple rule, we pretty much get everything we want: a nice schema definition with minimal SDL changes (a single new keyword), no type artifacts in the query, and deterministic behavior relying on nothing more than the schema.

inputUnion MetricInputUnion = SummaryMetric | CounterMetric | HistogramMetric

input SummaryMetric {
  name: String!
  min: Float!
  max: Float!
  count: Integer
}
input CounterMetric {
  name: String!
  count: Integer!
}
input HistogramMetric {
  name: String!
  percentiles: [Integer]!
  width: Integer
}

{name: "response.time", min: 123.4, max: 456.7, count: 89}
{name: "http.requests", count: 789}
{name: "response.time", percentiles: [12, 34, 56, 78, 90]}

If you wind up wanting two types that match exactly, this rule would push your design to contain enough fields to properly resolve the types. As far as I can tell, that's a good thing.

The auto-complete story isn't quite as simple as the rest of GraphQL, but it's definitely possible. To start, the interface could display a list of the possible fields grouped by type. As a field is chosen, the list of possible types is simply narrowed to those that still match.

The only downside I can currently think of is that sometimes you might actually want to have a field that maps 1-1 to the type, like an Enum. We wouldn't have a way to enforce that the value of a field matches the type:

{metricType: SUMMARY, name: "response.time", min: 123.4, max: 456.7, count: 89}
{metricType: COUNTER, name: "http.requests", count: 789}
{metricType: HISTOGRAM, name: "response.time", percentiles: [12, 34, 56, 78, 90]}

Probably not that big of a deal. Does anyone see any other issues that might arise with this solution?

[frankdugan3]

@binaryseed 1) Wouldn't this fall apart in the case where only the shared field was being provided? 2) there is value in separating types for logical reasons and future-proofing even if different types have the exact same fields. For example, ID is just a string, but it is useful to maintain a semantic difference. I see the most common use case for input unions is to match interface implementations, which do not have the restriction of requiring distinct fields.

[treybrisbane]

@binaryseed

Each type in the input union must have a unique set of required fields

This would have to be structurally recursive to be effective. I can imagine some not-so-nice scenarios in which a deeply nested input type ends up being the "deciding type" for an input union many layers above, leading to potentially very confusing breakages when such types are changed. Not sure how likely that kind of scenario is in practice though.

In any case, what you're suggesting would not address my particular use case, which is representing an AST in GraphQL:

input AndExpressionInput {
  lhs: ExpressionInput!
  rhs: ExpressionInput!
}

input OrExpressionInput {
  lhs: ExpressionInput!
  rhs: ExpressionInput!
}

# ...more types...

inputUnion ExpressionInput = AndExpressionInput | OrExpressionInput | # ...more types...

AndExpressionInput and OrExpressionInput are identical in structure, yet have fundamentally different meanings. The same can be said for pretty much any binary operator expression that I can think of.

More generally though, such an approach would also still leave us with an inconsistency between input and output unions. I personally see resolving this inconsistency as one of the primary benefits of this proposal.

[treybrisbane]

Potentially crazy idea:

Only require __inputname in cases where the server cannot unambiguously determine the appropriate input union "branch" for some client-submitted data. If __inputname is not supplied in that case, have the query fail validation.

Has a big downside in that adding a new structurally-identical type to an input union may break clients though, so probably not a good option.

[binaryseed]

Wouldn't this fall apart in the case where only the shared field was being provided?

I don't think so. The rule is that there is a unique set of required fields, so there can't be a case where only "shared" fields are provided...

This would have to be structurally recursive to be effective.

I don't see why this is true, can you explain? My proposal only states that a unique set of required fields is present, it doesn't say anything about what the types of those fields can be.

[jturkel]

@binaryseed - I believe your algorithm needs to take optional fields into account. See my comment for more details.

[treybrisbane]

@binaryseed

I don't see why this is true, can you explain? My proposal only states that a unique set of required fields is present, it doesn't say anything about what the types of those fields can be.

My interpretation of "field" is a "name, type pair". 😄

So the following would be a valid schema, because even though both Foo and Bar each have only a single field called id, the types of those fields are different:

input Foo {
  id: String!
}

input Bar {
  id: Int!
}

inputUnion FooBar = Foo | Bar

More generally, my interpretation was that we would need to include both the name and the type of each field in an input object type when comparing branches of an input union (in order to fully know if branches are identical). As a consequence of including field types in the comparison, the comparison becomes recursive, as we need to be able to determine whether two fields that have identical names but different input object types are equal.
Under my interpretation, the following schema would be invalid because the branches of the input union are structurally identical:

input FooData {
  name: String!
}

input BarData {
  name: String!
}

input Foo {
  data: FooData!
}

input Bar {
  data: BarData!
}

inputUnion FooBar = Foo | Bar

Am I correct in assuming you're talking about only the field names? If so, under your proposal, neither of those above schemas would be valid, but I'd consider that to be very restrictive.

[IdeaHunter]

@binaryseed

challenge to see if we can design an inputunion without a discriminator field, and I'd like to toss out a solution to see if it might work.

What about the case of nullable(optional) value types, for example int that have null value?
GraphQL can return from query discriminator field with null values with no sweat, for example

type FieldValue = DoubleValue | IntValue
type DoubleValue{
   key: String!
   value: Float
}
type IntValue{
  key: String!
  value: Int
}

in this case the only way to do figure out a difference is provide a function which can tell for a required field which value type should be expected

[binaryseed]

I created a PR to start a doc for further detailed discussion about possible solutions:

#584

[benjie]

I think we should revisit the tagged union pattern, discussed at the beginning of the thread, with the addition of a directive to require exactly one field be specified. To help with this, I've written it up as a straw-man: #586

[treybrisbane]

@benji Presumably you're suggesting this because you dislike one or more aspects of the current proposal... Can you elaborate on why you don't feel that the original proposal is worth pursuing?

From memory, I don't recall that approach having much (if any) substantive support during this discussion, so I admit to being a little confused as to why you feel we should revisit it. :S

[benjie]

Back when tagged unions was first proposed, there was a lot of hope that we’d find a clean solution to input unions that pleased everyone. Alas we have not; people are hung up (and rightly so, IMO) on __inputname, on performance costs of trying to dynamically determine the input type, and so on.

Tagged input unions already work, they just lack a tiny feature that would make them type safe. I’ve read through some of the disagreements and there seems to be a lot of hand-wavey things, like claims of decreased performance, aesthetics, etc. I cannot see how it would be any less performant than searching a chain of possibilities at runtime to determine the type. Further, it enables the scalar/object alternatives that a few people have wanted, that inputUnion cannot seem to give us.

Visiting the guiding principles of GraphQL spec changes, tagged unions with an added directive to enable type safety ticks more of the boxes than inputUnion; it achieves the goal with significantly less implementation overhead, is much more backwards compatible, and almost negligible performance overhead. As the guidelines say: “There are plenty of behaviors and patterns found in other languages intentionally absent from GraphQL. "Possible but awkward" is often favored over more complex alternatives.”

I believe, now that we know our options for inputUnion, we should revisit this enhanced proposal and evaluate it critically versus inputUnion, keeping in mind that there’s nothing stopping us adding actual input unions in future should we so desire.

I truly believe that the tagged union pattern with added directive for type safety is the best option available to us currently.

[acao]

As I mentioned in the WG meeting, here's a general utility script I created tonight that handles building all the linkages for you, if anyone wants to iterate on another fork of the graphql-js reference implementation of their own for the various proposals here, all the way up to graphiql:

https://www.npmjs.com/package/yarn-compose

the readme demonstrates a config for re-creating the setup I described above, using the same branches that were used in the graphql working group demo.

[acao]

well put @benjie, I like this proposal a lot, and not just because of how simple it'll be to implement for various runtimes and for the IDE ecosystem(s). I'm torn between this and the discriminator oriented proposals personally, but I'm glad to help advance whatever proposals folks want to prototype and/or officially adopt in the long run.

I just wanted to say, this has been a really amazing effort so far. We have the opportunity to learn from prior art and the mistakes of other API standards. Whatever comes of this deliberation and RFC process will open up so many new possibilities in the GraphQL community!

[bilby91]

Hello!

What is the current state of this RFC ? Is it waiting for something ?

Thanks!

[dmitry]

@bilby91 looks like it was merged into the RFCs in graphql-spec repo: https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md

[jodinathan]

@bilby91 looks like it was merged into the RFCs in graphql-spec repo: https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md

Sorry, but does this mean it is official now?
I would like to use this soon

[leebyron]

The files in RFC are collaborative RFC. Since multiple people have been contributing it was merged into that folder so additional people can work on it. It’s “official” (final stage) once it has been merged into the specification document itself.

The last two working group meetings have not had agenda items from champions for this RFC, so I think you should reach out to the RFC authors to offer your help if you would like to see things move quicker.

[IvanGoncharov]

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.

[mansona]

For anyone trying to follow that (now broken) link: the latest discussion on this topic is #825 👍