Consolidate Input Union discussions

[binaryseed]

Over the years there have been many discussions around the topic of Input Unions in GraphQL. To help further the effort, we have begun building a single RFC document. The goal of this document is to consolidate all related ideas to help move the proposal forward.

• https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md

To help this effort, I'd like @leebyron to consider commenting on & closing all related open Issues / PRs in favor of the main RFC document, which will contain & reference all of these previous discussions.

• Polymorphic input types Polymorphic input types #114 @luisobo
• RFC: inputUnion type RFC: inputUnion type #395 @tgriesser
• Input polymorphism Input polymorphism #415 @jayhasyee
• [RFC] GraphQL Input Union type [RFC] GraphQL Input Union type #488 @frikille
• [RFC] Input Objects accepting exactly @onefield [RFC] Input Objects accepting exactly @oneField #586 @benjie

@leebyron @IvanGoncharov

[dendrochronology]

Thanks for all your hard work in consolidating this @binaryseed, @benjie, et al!

To whom can I send virtual beers so this gets on the working group's agenda? 🍻

[benjie]

Thanks for the support and offer @dendrochronology; we discuss this frequently at working groups so it’s definitely already on our agenda :) We’re aiming to have the RFC doc updated by the next WG to enable participants to have a more informed discussion. It seems that there is no perfect solution (at least we haven’t found one yet!) so now it’s time to weigh up the different proposals and find out which trade-offs we can/can’t live with, which ones are the easiest to implement cross-platform, and which features we need/want/don’t care about.

[binaryseed]

@leebyron Would you mind taking a look at this?!

[IvanGoncharov]

@binaryseed Terribly sorry for the delay.
I closed all the issues and will review all pending PRs to RFC tomorrow.
Anything else I can do before closing this issue?

[akomm]

I refer to this answer, I do not want to continue in the closed topic.

@benjie I have more and more the feeling, that the current graphql enumeration should have been more powerful in terms of its allowed values and work similar to how the proposed oneField works. Basically like the enums you have in rust (or similar languages ...).

The advantage is clear, you do not need an artificial discriminator. Neither on the input, nor output side. The option (=field) is the discriminator by itself.

It might be considered something long-term, maybe for graphql v2, as an enum replacement. Short/mid-termn oneField would be as close as possible to it - technically for the client almost the same and easy to migrate.

[benjie]

@akomm Thanks for the feedback :) One issue that @oneField doesn't allow for that GraphQL interfaces do allow for is spreading a shared fragment (e.g. an interface fragment) over a mixture of types. For example if you had products in a supermarket you might have:

interface Product {
  name: String!
  priceInCents: Int!
}
type Clothing implements Product {
  # ...
  size: String
}
type Food implements Product {
  # ...
  calories: Int
}
type Cleaning implements Product { ... }

type Query {
  search(query: String): [Product]
}

you might issue this request:

{
  search(query: "cream") {
    __typename
    ... on Product { name priceInCents }
    ... on Clothing { size }
  }
}

This allows you to display a common search interface for all your products displaying name and price; but for certain "supported" products you could enhance the display with extra metadata - e.g. size for clothing or calories for food. I'm not sure how you'd achieve this in the @oneField solution.

I think @oneField could have worked instead of unions; however the difference between unions and interfaces may have been odd. Ultimately I think we've got a fine solutions for output types (especially with the enhancements to interfaces that are currently being merged) but input types are a different beast and have different needs IMO.

[akomm]

@benjie is it really simpler in a practical situation? I think not only the query should be considered, but also working with the result on the client. The advantage in your example is that you get the data as a flat collection. But you also have to discriminate by a string value on __typename. With oneField you would do the same via the field. In both cases when you render data you will have some sort of delegation of the refined/discriminated type on the client side. Something like renderClothing, renderCleaning or let's say in react <Clothing /> and <Cleaning />. The difference here would be how you pass the data:

if (result.clothing) {
  return <Clothing data={result.clothing} />;
}
if (result.cleaning) {
  return <Cleaning data={result.cleaning} />;
}

vs.

switch (result.__typename) {
  case 'Clothing':
    return <Clothing data={result} />;
  case 'Cleaning':
    return <Cleaning data={result} />;
}

For this to properly type-check with typescript, the typing extracted from introspection must provide the __typename as a string literal union of all possible type names that implement the interface. I am not sure if that is possible via introspection. I will check it out asap. But for example the current implementation of relay-compiler with typescript language plugin does only expose the __typename as a string.

Comparison type emission for both interface & oneField:

interface Product {}
interface Clothing extends Product {}
interface Food extends Product {}
interface Cleaning extends Product {}
// This is needed for oneField
type Result = 
  | {clothing: Clothing}
  | {cleaning: Cleaning}
  | {food: Food}
// vs __typename that uses string literal union based on type names
type Result = 
  | ({__typename: 'Clothing'} & Clothing)
  | ({__typename: 'Cleaning'} & Cleaning)
  | ({__typename: 'Food'} & Food)

Actually, with strict setting the oneField one would not work. You would have to define it either like this:

type Result =
  | ({clothing: Clothing} & {cleaning?: undefined, food?: undefined})
  | ({cleaning: Cleaning} & {clothing?: undefined, food?: undefined})
  | ({food: Food} & {cleaning?: undefined, clothing?: undefined})

With ever growing list... Or alternative, make assertions, which is very ugly and can be done different ways, I do not even try it here... :)

As for the query. I would like to compare how you could do both with more data fetched to make clear, where the difference is:

query ProductSearchQuery {
  search {
    clothing {
      ...ProductData
      size
    }
    cleaning {
      ...ProductData
    }
    food {
      ...ProductData
      calories
    }
  }
}
fragment ProductData on Product {
  name
  priceIncents
}

vs

query ProductSearchQuery {
  search {
    __typename
    ... on Product {
      name
      priceIncents
    }
    ... on Clothing { 
      size 
    }
    ... on Food { 
      calories
    }
  }
}

But this is just the js/ts side of the client. There are obviously other languages to be considered where the picture might be a different one...

[benjie]

You raise an interesting point: should @oneField exclude the other fields entirely rather than returning nulls? I'd always imagined it returning nulls, but perhaps omitting the fields in the same way the @skip directive would makes sense. Then your type is just type Result = {clothing: Clothing} | {cleaning: Cleaning} | {food: Food} even in more strongly-typed languages than TypeScript.

Note that the Product interface could be implemented by hundreds of types, and you may only have special treatment for a few of them (Clothing, Food). With my code you can add more types to the Product interface later and without having to update your client or change the query your application will continue to work because it can rely on name and priceInCents always being present, even on types it didn't know about when it was released. This is one of the superpowers of interfaces.

[akomm]

The issue with the typing in TS with fully strict setting is, that you can not check for the variants. The keys can only be checked against when they are explicitly defined, even if the only possible value is null or undefined.

interface Product {}
interface Clothing extends Product {}
interface Food extends Product {}
interface Cleaning extends Product {}
// This is needed for oneField
type Result =
  | {clothing: Clothing}
  | {cleaning: Cleaning}
  | {food: Food}
 
function render(result: Result) {
  // TS2339: Property 'clothing' does not exist on type 'Result'.
  // Property 'clothing' does not exist on type '{ cleaning: Cleaning; }'.
  if (result.clothing) {} 
}

You would have to make assertions/type-guards for every product type.

[Janpot]

@akomm How about:

type Result =
  | {food?: null, cleaning?: null, clothing: Clothing}
  | {food?: null, cleaning: Cleaning, clothing?: null}
  | {food: Food, cleaning?: null, clothing?: null}

It's a bit verbose but I guess that wouldn't be a problem if it's generated, right?

[akomm]

@Janpot which is basically what I have posted here, refined further.

Even if it is generated, it might be an issue because TS will not give you a readable error. Imagine having 100 or more different products with different properties. Also not sure what implication it would have on TS performance if the type gets ridiculous large. At this point, I do not want to get away from the main topic, so to cut in in general: I think that is should not be the desired outcome to have to generate such types ;)

[benjie]

You could do it like:

type Result_void = { food?: null, cleaning?: null, clothing?: null };
type Result =
  | { food: Food } & Omit<Result_void, 'food'>
  | { cleaning: Cleaning } & Omit<Result_void, 'cleaning'>
  | { clothing: Clothing } & Omit<Result_void, 'clothing'>;

but yeah, it's not super elegant. You could also use auto-generated type guards, but that's not great either. It's a shame TypeScript doesn't seem to have better support for this pattern, because in vanilla JS it'd be absolutely fine.

[Janpot]

Looks like RequireOnlyOne from this SO answer would do the trick

type RequireOnlyOne<T, Keys extends keyof T = keyof T> =
  Pick<T, Exclude<keyof T, Keys>>
  & {
    [K in Keys]-?:
      Required<Pick<T, K>>
      & Partial<Record<Exclude<Keys, K>, undefined>>
  }[Keys];


type Result = RequireOnlyOne<{ food: Food, cleaning: Cleaning, clothing: Clothing }>

[benjie]

[Action from GraphQL Spec WG]: I volunteer to champion solution 5 (the @oneField / @oneOf directive). Spec edits, including the effects on schema introspection, can be seen here: https://github.com/graphql/graphql-spec/pull/586/files

[binaryseed]

@benjie Could you start a PR that summarizes the introspection changes needed for solution 5 into the RFC doc?

[benjie]

Sure; I’ll try and get that done in the next week or two.

Update: submitted: #660

[benjie]

@binaryseed Would you be open to a PR that replaces all the inline links with reference-style links; e.g.

|    | [1](#-1-explicit-__typename-discriminator-field) | [2](#-2-explicit-configurable-discriminator-field) | [3](#-3-order-based-discrimination) | [4](#-4-structural-uniqueness) | [5](#-5-one-of-tagged-union) |
| -- | -- | -- | -- | -- | -- |
| [K](#-k-input-unions-should-be-expressed-efficiently-in-the-query-and-on-the-wire) | ❔ | ❔ | ❔ | ❔ | ❔ |
| [L](#-l-input-unions-should-be-performant-for-servers) | ❔ | ❔ | ❔ | ❔ | ❔ |
| [M](#-m-existing-sdl-parsers-are-backwards-compatible-with-sdl-additions) | ❔ | ❔ | ❔ | ❔ | ❔ |
| [N](#-n-existing-code-generated-tooling-is-backwards-compatible-with-introspection-additions) | ❔ | ❔ | ❔ | ❔ | ❔ |

would become:

|    | [1][solution-1] | [2][solution-2] | [3][solution-3] | [4][solution-4] | [5][solution-5] |
| -- | -- | -- | -- | -- | -- |
| [K][criteria-K] | ❔ | ❔ | ❔ | ❔ | ❔ |
| [L][criteria-L] | ❔ | ❔ | ❔ | ❔ | ❔ |
| [M][criteria-M] | ❔ | ❔ | ❔ | ❔ | ❔ |
| [N][criteria-N] | ❔ | ❔ | ❔ | ❔ | ❔ |

[criteria-K]: #-k-input-unions-should-be-expressed-efficiently-in-the-query-and-on-the-wire
[criteria-L]: #-l-input-unions-should-be-performant-for-servers
[criteria-M]: #-m-existing-sdl-parsers-are-backwards-compatible-with-sdl-additions
[criteria-N]: #-n-existing-code-generated-tooling-is-backwards-compatible-with-introspection-additions

[solution-1]: #-1-explicit-__typename-discriminator-field
etc

(And all the links in the rest of the document would get cleaner too.)

[binaryseed]

Would those be clickable links still?

I don't care much what the markdown looks like for those, really what matters is that they help navigate around the document

[benjie]

Yeah, they work exactly the same (the output HTML is identical) it just makes it easier to read/edit the markdown.

[binaryseed]

then yes, sounds great!

[sabit990928]

Hi guys!
I read this https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md#-5-one-of-tagged-union and as I understand we can only use last one(https://github.com/graphql/graphql-spec/blob/master/rfcs/InputUnion.md#-5-one-of-tagged-union ) to solve problems?
Because any other variant has not solution for M. Existing SDL parsers are backwards compatible with SDL additions(inputunion field). I'm running code in Node.JS env. Mongoose as a DB.

Or we can write inputunion in some other way. Spent so much time by searching an answer)
Actually Third one in the list was a good solution in my case, but inputunion......

[ddevault]

Stopping in to share my thoughts on the RFC, as I just ran into this while writing up my owl gql schema. The reason I'm chiming in is that (1) I think my use-case is unique among those discussed so far and (2) it provides a use-case for criteria G ("Input unions may include other input unions").

In my case, I want to use unions as a kind of more-sophisticated ID. On my system, users are either uniquely identified by an integer ID, or by their username. Users also own repository resources, which can themselves by uniquely identified by an integer ID, or by a combination of user and repository name. To make things convenient on API users, I want to give them all of these options for unambiugously specifying a repository. Therefore, I have the following schema:

# Entity canonical name (e.g. "~sircmpwn") or integral ID, either uniquely
# identifies an entity
union EntityID = String | ID

# A combination of a repository's owner entity ID and its name
type OwnerRepo {
  owner: EntityID!
  repoName: String!
}

# An OwnerRepo or an integral ID, either uniquely identifies a repository
union RepoID = OwnerRepo | ID

type Query {
  # Returns a specific repository
  repository(id: RepoID): Repository
}

[binaryseed]

Closing since these discussions were consolidated in the RFC document!

[ddevault]

Can you link to the most current RFC document and the appropriate place for its discussion, @binaryseed? This issue was already supposed to consolodate several and now it's fractured again...

[binaryseed]

The RFC is linked in the issue description, and discussions can happen through PRs to that document!

[gonzaloserrano]

For the casual reader, looks like the action is happening here #733

[BatmanAoD]

Looks like the currently-open discussion is here: #825