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

Literal types support #878

Open
PinkaminaDianePie opened this issue Jul 18, 2021 · 34 comments
Open

Literal types support #878

PinkaminaDianePie opened this issue Jul 18, 2021 · 34 comments

Comments

@PinkaminaDianePie
Copy link

Consider adding literal types support to be able to express a union with more than a single discriminator:

type Sword {
  weaponType: "melee"!
  aptitude: "neutral"!
  damage: Float!
}

type Rifle {
  weaponType: "ranged"!
  aptitude: "technological"!
  damage: Float!
}

type Spellbook {
  weaponType: "magic"!
  aptitude: "magical"!
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

__typename is useless in a case where multiple discriminators are needed. Multiple discriminators allow us to write more type-safe code, using exhaustive type check.
Currently, the only partial alternative is to use enums:

enum WeaponType {
  melee
  ranged
  magic
}

enum ItemAptitude {
  neutral
  technological
  magical
}

type Sword {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
}

type Rifle {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
}

type Spellbook {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

The problem is that it doesn't allow to use of different properties as union discriminators. Consider such a react component:

const WeaponFrame = ({weapon, children}: Props): JSX.Element => {
  switch (weapon.type) {
    case 'melee':
      return <MeleeWeaponFrame>{children}</MeleeWeaponFrame>;
    case 'ranged':
      return <RangedWeaponFrame ammo={player.ammo}>{children}</RangedWeaponFrame>;
    case 'magic':
      return <MagicWeaponFrame charges={weapon.chargesLeft}>{children}</MagicWeaponFrame>;
    default:
      ((_: never) => _)(weapon.type);
      throw new Error('unreachable case');
  }
}

here I know that if type of weapon is magical, I need to display chargesLeft property. however, it's impossible to represent it in graphql, so it's also impossible to generate a typescript type from a schema.

The only solutions left is to give up on type safety and hope that every developer won't forget to check every case, or just give up on generating types from the graphql schema and instead write all of them manually, casting graphql query results to manual typings on API level. Any of these solutions have multiple points of failure, so there is no easy and safe way to represent such kind of data at this moment.

@n1ru4l
Copy link

n1ru4l commented Jul 18, 2021

You could model those literal types using interfaces:

interface MagicalItem {
  magic: Boolean!
}
interface RangedItem {
  ranged: Boolean!
}
interface MeeleItem {
 meele: Boolean!
}
interface NeutralItemAptitude {
  neutral: Boolean!
}
interface TechnologicalItemAptitude {
  technological: Boolean!
}
interface MagicalItemAptitute {
  magical: Boolean!
}

type Sword implements MeeleItem, NeutralItemAptitude {
  meele: Boolean!
  neutral: Boolean!
  damage: Float!
}
type Rifle implements RangedItem, TechnologicalItemAptitude {
  ranged: Boolean!
  technological: Boolean!
  damage: Float!
}
type Spellbook implements MagicItem, MagicalItemAptitude {
  magic: Boolean!
  magical: Boolean!
}
union Weapon = Sword | Rifle | Spellbook

That would allow discriminating a generated union types with a property in object check in TypeScript.

if ('ranged' in item) {
  // thing must implement the RangedItem interface ☑️
}
if ('magic' in item || 'technological' in item) {
  // thing must implement either MagicItem or TechnologicalAptitude interface ☑️
}

@PinkaminaDianePie
Copy link
Author

@n1ru4l that's not a solution, because it doesn't allow us to use exhaustive type check === solution is not type-safe, and even less type-safe than creating the types manually and casting them manually on API level. Consider a case, when devs will need to add a new weaponType. In that case, all of the checks I did with using switch will trigger an error for them, so they will see all of the places they need to change. Same with removal of a particular weaponType - devs will see all of the places to fix, without even running a single test. An exhaustive type check forces you to cover all of the cases, so you won't accidentally forget it. Having multiple boolean flags gives nothing. It's more safe to create all of the types manually and cast graphql results to these types on API level, so only API part will be our point of failure. Using boolean flags will make the entire codebase unsafe, so it will be much harder to spot the bug then with type casts in a single place.

@n1ru4l
Copy link

n1ru4l commented Jul 19, 2021

Example code and "switch" alternative using if statements:

https://www.typescriptlang.org/play?#code/C4TwDgpgBAyg7gewE4BMoF4oG8BQUoC2EEANhAFxQBGCCZAhgHZ5SMQCuwS9JlNdEJixT0C9AOYVW7AlQhIcAXxyhIUAEoBLAGZl8mXPm6NJKPrQbN8wCAGMAFozoJxm2z3MCh+EWMmVGGTkFZVVoGEgSEn4AawxsFj83T0tEiTcPagtBKyhfCSlA2XkWB3okSQBnABkIbWAAoJLlFXBoAHVBMARGePhkNAAfDR09YYjSaNoYnBxbHsrgKCQIRhR5AFliMk76bt7MAAoiUghd-cp+1ABKDAA+BKMIYHYkXsCopVn5xkXl1fWSHUTFM5x68UOxlBXR6lC0uggt3QD0M-xeb2knxaPz+KzWm3StjBBygx0JxMukSmCBiSJRLBW6Pe7Cx3wWS0kwAAIqIChC4DDGJRiddGlF7o8oDpSQAiE5kGVS3oCvY9W6op5M-74pBbU7Ew4q-bXFjKfDSw4yqEQFCKzTKwXqlia169PGA4EmG0Go1q01QBnPV3awEbcmCw2OpRAA

Your described case where a new weapon type is introduced:

https://www.typescriptlang.org/play?ssl=44&ssc=2&pln=1&pc=1#code/C4TwDgpgBAyg7gewE4BMoF4oG8BQUoC2EEANhAFxQBGCCZAhgHZ5SMQCuwS9JlNdEJixT0C9AOYVW7AlQhIcAXxyhIUAEoBLAGZl8mXPm6NJKPrQbN8wCAGMAFozoJxm2z3MCh+EWMmVGGTkFZVVoGEgSEn4AawxsFj83T0tEiTcPagtBKyhfCSlA2XkWB3okSQBnABkIbWAAoJLQ8GgAUQAPMBIESs0AN2gDFggunr7BlJzhUQLG4pCcFVaoAHVBMARGePhkNAAfDR09Q4jSaNo4w87u3oGIJdstyuAoJAhGFHkAWWIydfom22mAAFERSBAAUDKLtUABKDAAPgSRggwHYSG2gSiSkez1e70+8nUTFMUK28RBxjJGy2lC0uggCPQyMMbzRGKx7BxyhwT0YL3ZRKQ33StnJwKgYLFEphkQuCBizNZLHe6Mx0h5eIFr0kwAAIrNJJS4LTGJQJXDGlEkSioDopQAicFkR327amwFbBFs1Hq7aEr4iv6Qs0gz1AuEsZT4B0gx3UiAoN2aD1mn0sP2coVBkkmJMS8Pp6NQVUcjWBn4ysMR71KIA

As you can see the introduction of a new type results in a TypeScript error.

Please also differentiate between "not the optimal solution that you desire" and "not a solution".

As mentioned on Discord before, there is definitely some room for improvement here and literal types might be a possible solution for making this more convenient.

@PinkaminaDianePie
Copy link
Author

now open your second link and change the union to type Weapon = Sword | Spellbook. type check will pass, while it clearly should trigger an error. exhaustive type check should work in both directions, not only "when things are right".

if the solution requires to change the data model, introduce lots of unnecessary fields, throws away standard TS way to iterate over union but fails to detect the error anyway - that's not "not the optimal solution", that's just "not a solution"

@n1ru4l
Copy link

n1ru4l commented Jul 19, 2021

Regarding the proposal, could the introduction of exact enum field types be a possible solution?

enum WeaponType {
  melee
  ranged
  magic
}

enum ItemAptitude {
  neutral
  technological
  magical
}

type Sword {
  weaponType: WeaponType.melee!
  aptitude: ItemAptitude.neutral!
  damage: Float!
}

type Rifle {
  weaponType: WeaponType.ranged!
  aptitude: ItemAptitude.technological!
  damage: Float!
}

type Spellbook {
  weaponType: WeaponType.magic!
  aptitude: ItemAptitude.magical!
  damage: Float!
  chargesLeft: Int!
}

@PinkaminaDianePie
Copy link
Author

In my particular case, exact enums would solve all the issues I have. The only problem is that it works only with strings, but I saw some cases where people used number/boolean literals as discriminators. Not sure how common is this case, but that's probably a separate topic - allowing enum to have boolean/numeric values is a nice thing to have, but not mandatory to solve the issue we talk about. I'd be happy to see either solution

@mdecurtins
Copy link

+1 Exact enums would be a useful construct, particularly in the absence of generics.

@n1ru4l
Copy link

n1ru4l commented Mar 31, 2022

I created this branch where I added parser and printer support for exact enums, it was a fun experience diving into the parser and lexer: https://github.com/graphql/graphql-js/compare/main...n1ru4l:feat-exact-named-type-node?expand=1

This is still far from an RFC, but I am just experimenting a bit with it for now!

@yaacovCR
Copy link
Contributor

yaacovCR commented Apr 1, 2022

Would a Union of Interfaces help? You can use the schema to automatically generate type predicates that check whether a received concrete type satisfies an interface based simply on the type name. This is similar to suggestion above, but avoids booleans

@n1ru4l
Copy link

n1ru4l commented Apr 1, 2022

@yaacovCR One drawback of that solution is that the functions can become quite large if there are a lot of types within the schema.

This solution would also work with the boolean solution (but less verbose as type names tend to be longer).

In contrast, the exact enums approach would not require generating any function code.

@yaacovCR
Copy link
Contributor

yaacovCR commented Apr 1, 2022

Although I am in favor of literals in general mostly because of why not

@yaacovCR
Copy link
Contributor

yaacovCR commented Apr 4, 2022

@yaacovCR One drawback of that solution is that the functions can become quite large if there are a lot of types within the schema.

This solution would also work with the boolean solution (but less verbose as type names tend to be longer).

In contrast, the exact enums approach would not require generating any function code.

True. To clarify, you would need a generated predicate per interface, not per concrete implementation.

The good news is that you would have symmetry between your runtime types and the graphql types instead of so that could be an organizational plus.

Those predicates might be handy anyway. Imagine if you already had such an interface setup, for example. We are talking about introducing it to solve this problem, but I bet a lot of schemas already use interfaces, and such predicates would also help them.

Maybe TypedDocumentNode would already solve this problem also...

@PinkaminaDianePie
Copy link
Author

I don't understand how the union of interfaces would help. I want to specify that some particular interface has a field with literal type as a value. It doesn't matter if we have types or interfaces, autogenerated code, and so on, I just want to restrict the value of one particular field to a literal on a schema level to generate proper TS types and I can't do it.

@yaacovCR
Copy link
Contributor

yaacovCR commented Apr 4, 2022

It wouldn't help you have a literal type for a field, but it would help you create a system for discriminating between related types on the client exhaustively using TS typings -- in this case identical to native graphql types. I thought that was the end goal.

@n1ru4l
Copy link

n1ru4l commented Apr 4, 2022

@yaacovCR Can you provide an example? It is really hard to grasp what you actually mean. When i try to interpret it it feels like what you are describing is the same as #878 (comment)

@yaacovCR
Copy link
Contributor

yaacovCR commented Apr 4, 2022

@n1ru4l sort of -- I assumed that I could solve it using interfaces without realizing that we don't have empty marker interfaces and so we can't have an interfaces for RangedWeapon NeutralItem without dummy fields.

I think that solution with using __typename would be type-safe, but without empty composite types, it probably is not that useful.

Apologies for the distraction.

@rivantsov
Copy link
Contributor

__typename is useless in a case where multiple discriminators are needed.

can you please expand on this? and why this:

   switch (weapon.__typeName) { ... 

wouldn't work? why multiple discriminators are needed?

@n1ru4l
Copy link

n1ru4l commented Apr 5, 2022

@rivantsov The first post shows a good example: #878 (comment)

union Weapon = Sword | Rifle | Spellbook

If all of those types have different properties (weaponType, aptitude) it is impossible to discriminate them based on only one or both of those properties. Using the typename allows to only safely discriminate one property (the type itself).

@rivantsov
Copy link
Contributor

sorry but still not getting it. Why __typeName not discriminates enough?!

@n1ru4l
Copy link

n1ru4l commented Apr 5, 2022

@rivantsov Today we only have one value (aside from doing structural checks for identifying what a type actually is).

enum WeaponType {
  melee
  ranged
  magic
}

enum ItemAptitude {
  neutral
  technological
  magical
}

type Sword {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
}

type Rifle {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
}

type Crossbow {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
}

type Spellbook {
  weaponType: WeaponType!
  aptitude: ItemAptitude!
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Crossbow | Spellbook

This does not allow us to get the exact value of the weaponType or aptitude properties through introspection. Code generation tools can thus not generate code that allows making usage of Discriminating Unions for those.

E.g. the Crossbow.weaponType field of the (enum) type WeaponType will always resolve to ranged. Today we don't have a way of expressing such behavior. Having a construct that allows this would help tooling.

The TypeScript representation of the example above could today only be expressed as the following:

type WeaponType = "melee" | "ranged" | "magic";
type ItemAptitude = "neutral" | "technological" | "magical";

type Sword = {
  __typename: "Sword";
  weaponType: WeaponType;
  aptitude: ItemAptitude;
  damage: number;
};

type Rifle = {
  __typename: "Rifle";
  weaponType: WeaponType;
  aptitude: ItemAptitude;
  damage: number;
};

type Crossbow = {
  __typename: "Crossbow";
  weaponType: WeaponType;
  aptitude: ItemAptitude;
  damage: number;
};

type Spellbook = {
  __typename: "Spellbook";
  weaponType: WeaponType;
  aptitude: ItemAptitude;
  damage: number;
  chargesLeft: number;
};

type Weapon = Sword | Rifle | Crossbow | Spellbook;

Where as the literal type/exact enum type approach (from #878 (comment)) would allow generating types where the weaponType and aptitude properties can be narrowed down to the actuel runtime type.

Schema with exact enum types

enum WeaponType {
  melee
  ranged
  magic
}

enum ItemAptitude {
  neutral
  technological
  magical
}

type Sword {
  weaponType: WeaponType.melee!
  aptitude: ItemAptitude.neutral!
  damage: Float!
}

type Rifle {
  weaponType: WeaponType.ranged!
  aptitude: ItemAptitude.technological!
  damage: Float!
}

type Crossbow {
  weaponType: WeaponType.ranged!
  aptitude: ItemAptitude.neutral!
  damage: Float!
}

type Spellbook {
  weaponType: WeaponType.magic!
  aptitude: ItemAptitude.magical!
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Crossbow | Spellbook

typescript output

type Sword = {
  __typename: "Sword";

  weaponType: "melee";
  aptitude: "neutral";
  damage: number;
};

type Rifle = {
  __typename: "Rifle";

  weaponType: "ranged";
  aptitude: "technological";
  damage: number;
};

type Crossbow = {
  __typename: "Crossbow";

  weaponType: "ranged";
  aptitude: "neutral";
  damage: number;
};

type Spellbook = {
  __typename: "Spellbook";

  weaponType: "magic";
  aptitude: "magical";
  damage: number;
  chargesLeft: number;
};

type Weapon = Sword | Rifle | Crossbow | Spellbook;

This allows more powerful type narowing:

function printWeapon(weapon: Weapon) {
  if (weapon.weaponType === "ranged") {
    // weapon is narrowed down to Rifle or Sword
  }
  // instead of doing
  if (weapon.__typename === "Rifle" || weapon.__typename === "Crossbow") {
    // weapon is narrowed down to Rifle or Sword
  }
  // or
  if (weapon.weaponType === "neutral") {
    // weapon is narrowed down to Crossbow or Sword
  }
  // instead of
  if (weapon.__typename === "Sword" || weapon.__typename === "Crossbow") {
    // weapon is narrowed down to Crossbow or Sword
  }
}

Thus the arguments for introducing it into GraphQL would be:

  • more flexible narrowing for tooling that uses introspection for code generation
  • more type-safety for developers that built upon GraphQL APIs

Whether it shall be introduced as exact enums or actual literal types is still somethign to discuss though 😄

@rivantsov
Copy link
Contributor

Suggesting simpler alternative: Constant Fields

thanks for the explanation. Now I get it, kind of.
So to sum it up - you need is a way to express that a field WeaponType on SpellBook object will always return value MAGIC. And make this fact discoverable by tools thru introspection or by reading a schema file.

But then what you need is not 'literal types' but Constant fields , or fields with 'default-forever-value'. So instead of:

type Spellbook {
  weaponType: WeaponType.magic!
  ... ... 
}

we write:

type Spellbook {
  weaponType: WeaponType! = Magic
  ... ... 
}

'Literal type' is an advanced type concept, not widespread, I had to google to find out WTF is that, found references to TypeScript and C++. I might be a dumb expert (I am), not knowing ALL stuff, but that makes me a good test case here - not everybody is familiar with literal types concept. It took me a while to get it what you suggest and how it will work.

At the same time, 'constant fields' is an easy, trivial concept to understand for everybody, even a dummy like myself
Yes, extra field(s) needed in introspection, but this the same as with literal types

@PinkaminaDianePie
Copy link
Author

PinkaminaDianePie commented Apr 6, 2022

constant fields is a harder concept than literal types and it makes things more complicated. why do I need to create enum just to express that my field has some specific value?
With literal types it would look like this:

type Sword {
  weaponType: "melee"!
  aptitude: "neutral"!
  damage: Float!
}

type Rifle {
  weaponType: "ranged"!
  aptitude: "technological"!
  damage: Float!
}

type Spellbook {
  weaponType: "magic"!
  aptitude: "magical"!
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

with constant fields it would look like this:

enum WeaponType {
  melee
  ranged
  magic
}

enum ItemAptitude {
  neutral
  technological
  magical
}

type Sword {
  weaponType: WeaponType! = melee
  aptitude: ItemAptitude! = neutral
  damage: Float!
}

type Rifle {
  weaponType: WeaponType! = ranged
  aptitude: ItemAptitude! = technological
  damage: Float!
}

type Spellbook {
  weaponType: WeaponType! = magic
  aptitude: ItemAptitude! = magical
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

it takes much more code and it also requires creating 2 additional enums I will never need in my code. Of course, I can create a dummy enum for all literal values like this:

enum Literal {
  melee
  ranged
  magic
  neutral
  technological
  magical
}

type Sword {
  weaponType: Literal! = melee
  aptitude: Literal! = neutral
  damage: Float!
}

type Rifle {
  weaponType: Literal! = ranged
  aptitude: Literal! = technological
  damage: Float!
}

type Spellbook {
  weaponType: Literal! = magic
  aptitude: Literal! = magical
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

but still why create an additional enum and have more code than needed? yes, ultimately it would solve the purpose to generate proper TS types from such a schema, but constant fields looks like unnecessary complexity

Edit: I just realized that nobody forces me to use enums here :D

type Sword {
  weaponType: String! = melee
  aptitude: String! = neutral
  damage: Float!
}

type Rifle {
  weaponType: String! = ranged
  aptitude: String! = technological
  damage: Float!
}

type Spellbook {
  weaponType: String! = magic
  aptitude: String! = magical
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

So version with constants is actually just a few characters longer that with literals

@rivantsov
Copy link
Contributor

So version with constants is actually just a few characters longer that with literals

but much simpler conceptually
In your last sample, with strings, I think values should be in double quotes

@rivantsov
Copy link
Contributor

rivantsov commented Apr 6, 2022

by the way, the same can be achieved today, with a custom directive:
@ const(str: value)

@PinkaminaDianePie
Copy link
Author

I can't agree that constants is a simpler context though. I speak from a frontend perspective (TS/Flow), where literal types are widely used (in all the codebases I worked with they are even more common than enums). Constants still require some additional mental work: "first we declared type as a set of values, but then narrowed it down to a single value" vs "we declared type as a single value". Why should I narrow it down, if I can properly specify it straight away? What this narrowing down implies? Is it safe to assume that here weaponType: String! = "magic" weaponType can be a "magic"? If so, why do we specify it as a sting? What = means? In lots of languages it means "default value", not a constant. And so on and so on. But when I see weaponType: "magic"! it's pretty straightforward. It means that weaponType can only accept a single value, "magic". No need to narrow anything down in the thought process, no need to think about meaning of = symbol, it's just simple

@n1ru4l
Copy link

n1ru4l commented Apr 6, 2022

by the way, the same can be achieved today, with a custom directive:
@ const(str: value)

There is only the limitation that the directives have no proper "on field type validation" and are not visible via introspection

@rivantsov
Copy link
Contributor

rivantsov commented Apr 6, 2022

directives on fields are not visible on fields -
then let's fix the introspection! it is long overdue and does not make sense that some things are not available in introspection. I tried to scream wtf, but was shutdown. It is nonsense

As for 'literal types' easy - good for you guys you spend most of your time in typeScript. I don't. I am a server side guy, never encountered it. The concept is not trivial. "ABC" is a type - whaaat? Constant fields are easier for sure. In c#, java etc, constants are static immutable fields with initial values.

What "=" means? same thing as for args with default values, this is in line with existing syntax.

@yaacovCR
Copy link
Contributor

yaacovCR commented Apr 7, 2022

For default values, you need the type because when supplied, the argument must match the type.

So the question becomes, with literals or constants, for the executor to validate the object against the type, is a type necessary?

I think it might be necessary for something like a constant custom scalar.

But if you just want to express the literal types that can be in sdl like string and it's equivalents (ID, enum) you wouldn't need it.

@itorres-1
Copy link

itorres-1 commented Mar 25, 2023

Hello all, kinda late to the convo and sadly, there hasn't been any activity on this for almost a year. Just wanted to share I found this because I too found myself needing GraphQL support for Typescript's discriminated unions, especially within the context of codegen tools like this one.

When I first started my project, I was manually maintaining TS types alongside a GraphQL schema that somewhat reflected these types but as my project began to grow, I found that this is a painstaking task and source of many bugs, so I decided to try and adopt usage of previously linked codegen tool. What I am finding is that discriminated unions are really difficult to model in a GraphQL schema, and the best idea I've had on how to do so is using custom scalars like:

scalar RootPostType
scalar ReplyPostType

type RootPost {
  type: RootPostType
  ...
}

type ReplyPost {
  type: ReplyPostType
  ...
}

And then, within my codegen.ts file, within the config section, I can add something like:

scalars: {
  RootPostType: "'root'",
  ReplyPostType: "'reply'",
},

However, as I'm still relatively new to GraphQL and currently using Apollo Server/Client tools, I've read I need to implement custom scalar logic, and since I'm using discriminated unions a fair bit, this might become quite a pain (not sure yet though).

I tried making use of the __typename property but I had a couple of issues with it:

  1. This is a property supplied only to the client (codegen tool does not create __typename for inputs), but my server would also sometimes need to distinguish types. For example, in my case above, when a user is submitting a root or reply post with different content inputs, they use a type like: union ContentInput = TextContentInput | LinkContentInput | ImageContentInput | ..., depending on the nature of the content, I'd like to process it differently when a root/reply post submission request is received server side. As it is, this already requires two separate mutation endpoints (if this is the right term) on the server side because again, I have no way of distinguishing whether a user is attempting to submit a root post or a reply post even though the logic is nearly identical.
  2. Even when it is supplied to the client, one of its potential values is undefined, and it's rather annoying having to first check that it isn't undefined and add logic to deal with the case when it is undefined even though I know this won't happen. Even when I do check that it isn't undefined, TS for whatever reason doesn't register the narrowing, like so:
    image Nevermind the toHexString() error, what I'm trying to show is that I've ruled out __typename being undefined, RootPostWithId and so the only possibility left is ReplyPostWithId which does contain the parentPath property and yet TS won't recognize it?

So, excuse the long post but I don't really want to rewrite a substantial part of my codebase that depended on discriminated unions so I'm hoping someone can suggest the best way to approach this problem or at the very least, be another scenario where GraphQL support for discriminated unions (however that may be, through literal types, exact enum types, or constant fields).

@haversnail
Copy link

Bumping this topic as it's been a minute.

Reading through the discussion above, @n1ru4l seems to have summarized the problem (and proposed solution) pretty well—is there any substantial objection to advancing this in the spec? If the concerns mentioned above are more semantic than functional in nature, I think both "literal types" and "constant fields" would be perfectly suitable names (naming is one of the hardest things, after all). 😉

Either way, the feature itself seems to fall into the "high impact/low effort" category. If it's just traction this needs, happy to help—we could definitely benefit from this too.

FWIW, I do think the dot notation (example A) does feel a bit more "natural" and universally understood than using the assignment operator/"equals" syntax (example B), as it can have notably different uses across languages, as @PinkaminaDianePie and @rivantsov both eluded to.

# Example A:
type Foo {
  bar: MyEnum.VALUE!
}

# Example B:
type Foo {
  bar: MyEnum! = VALUE
}

@justin-caldicott
Copy link

Any update on this? Discriminated unions are such a useful capability in typescript. When using GQL codegen to make typescript types we don't have this option, so we can't use it. Our codebase would really benefit from it.

Support for simple literal values as proposed by @PinkaminaDianePie would seem the most elegant to me:

type Sword {
  weaponType: "melee"!
  aptitude: "neutral"!
  damage: Float!
}

type Rifle {
  weaponType: "ranged"!
  aptitude: "technological"!
  damage: Float!
}

type Spellbook {
  weaponType: "magic"!
  aptitude: "magical"!
  damage: Float!
  chargesLeft: Int!
}

union Weapon = Sword | Rifle | Spellbook

@jasonkuhrt
Copy link

@justin-caldicott GraphQL built in a discriminant union on all object types, __typename.

@justin-caldicott
Copy link

Thanks for clarifying @jasonkuhrt. Being able to use any property as the discriminator, rather than just __typename would be helpful, as would allowing multiple discriminators. But for the case I'm looking at __typename should work fine.

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

No branches or pull requests

9 participants