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

Discriminated Unions - Multiple Fields, SubClassing/interfacings, kind field detection failure #27541

Closed
wesleyolis opened this issue Oct 4, 2018 · 5 comments
Labels
Discussion Issues which may not have code impact

Comments

@wesleyolis
Copy link

TypeScript Version: [email protected]
Search Terms:
Discriminated Unions - Multiple Fields

Issues:

When there are multiple common fields between interfaces the Discriminated Unions is not able to determine which field is to be the discriminator and all the logic that typescript will constrain the shape of the function interfaces bases on a field, check will not work.
It would be great if we could in cases like these define a common interface on which there is a single field present from which all other instance inherited from that shape. Then when one defines a union of types, if it can't find a single common field, then it inspects the base interfaces for an interface with a single field, if found, it will use that field from that interface. Seem to be present with many cases, were I need to communicate to typescript which field should be used.

There would still be a problem, if there is common class from which all inherited that only has a single field, in which case another form of syntax would be required when defined the discriminant, like including the common interface in the discriminate. This method, might be the preferable option, because it would be more performance than inspective all the interfaces form which a class extends from. I would choose this form, because simpler, as know exactly what to inspect at a top level.
See examples below of all the cases.
type Kinds = KindA | KindB | Kind

An improvement, would be to allow instanceof and similar variants to be used, versus string literals.

// just the common shape.
interface Kind {
    field : any
}
// or more specific
interface Kind {
    field : 'KindA' | 'KindB' // or any, should be good enough.
}

interface KindA extends Kind {
    field : 'KindA'
    fieldCommon : string;
    fieldA : 'KindAFieldA'
}

interface SubKindA extends KindA
{
    subFieldA : 'SubFieldKindA'
}

interface KindB extends Kind {
    field : 'KindB' 
    fieldCommon : string;
    fieldB : 'KindBFieldB'
}

interface SubKindB extends KindB {
    subFieldB : 'SubFieldKindB'
}


type Kinds = KindA | KindB | Kind // Is the other way to specify which interface the single field discriminator is defined in.

type Kinds = KindA | KindB
const kind = {} as Kinds;

switch(kind.field) // On field should be available, fieldCommon must not be.
{
    case 'KindA':
    kind.fieldCommon    
    kind.field
    kind.fieldA

    case 'KindB':
    kind.fieldCommon    
    kind.field
    kind.fieldB // Field should be available, but is missing
}

type SubKinds = SubKindA | SubKindB | Kind // Is the other way to specify which interface the single field discriminator is defined in.

type SubKinds = SubKindA | SubKindB
const subKind = {} as SubKinds;

// expect only fields with literals, should be available here, fieldCommon must not be
switch(subKind.field)
{
    case 'KindA':
    {
    subKind.field      
    subKind.fieldCommon 
    subKind.fieldA   
    subKind.subFieldA; 
    }

    case 'KindB':
    {
    subKind.field
    subKind.fieldCommon;   
    subKind.fieldB;          // should be available here, but is missing
    subKind.subFieldB;      // should be available here, but is missing
    }
}
@RyanCavanaugh RyanCavanaugh added the Discussion Issues which may not have code impact label Oct 4, 2018
@donabrams
Copy link

Forcing a user to always define a discriminator field has a runtime cost. It's possible to avoid needing a discriminator by analyzing the field/type combinations of each union member and building an efficient "given this object which union member are you?" query. It's definitely non-trivial but handles currently "surprising" cases like this MappedOmit<T, K> type:
https://gist.github.com/donabrams/b849927f5a0160081db913e3d52cc7b3

I'd rather have the compiler figure out the more efficient way to automatically discriminate types (and maybe warn if a type in a union is unable to be discriminated) than give the user a way to signal the discriminator.

@blixt
Copy link

blixt commented Nov 26, 2018

This issue is a bit hard to get into, maybe it could be written to gently get into the more deep issues? Am I correct in assuming that the root of many of these failures is simply described as the following?

type T1 = { a: number, b: number }
type T2 = { a: number, c: number }
type U = T1 | T2
const x: U = { a: 1, b: 2, c: 3 } // Does not implement either of the union'ed types!

In the above x should not be a valid value for U since it's not a valid value for T1 nor T2.

And for a more concrete use case: Imagine specifying an options object for a function (so it generally cannot have any literal values to use for discriminating the types) that must include one of two options, but never both. I haven't figured out a way to do this.

@wesleyolis
Copy link
Author

@blixt Typescript terminology use sets and logic is not as clear as you would think, but too late to change now fix now, because everything would break. You will just have to learn to work around the corner cases.

For what you want the following would work, which not discriminator types logic really, it is mix in different Record shapes into one larger one.

type T1 = { a: number, b: number }
type T2 = { a: number, c: number }
type U = T1 & T2 // Note that anding types would work here.
const x: U = { a: 1, b: 2, c: 3 } 

@blixt
Copy link

blixt commented Nov 28, 2018

@wesleyolis That's even further away from what I would want, because it creates a new type with a, b, c. I realize that they don't follow set semantics (because then T1 & T2 would be { a: number }), but I'm expecting T1 | T2 to be a logic for saying "either T1 or T2, but never both". That's exactly what it does if there's a literal in T1 or T2.

@donabrams
Copy link

donabrams commented Nov 28, 2018

There's a few different ways to think about these operations:

Operators as bitwise ops and types as Rows:
A | B == Row(A) | Row(B) == Row(A) ∪ Row(B)
ex: { a: 1 } | { b: 2 } is satisfied by { a: 1, b: 2 }
A & B == Row(A) & Row(B) == Row(A) ∩ Row(B)
ex: { a: 1, b: 2 } & { b: 2, c: 3} is satisfied by { b: 2 }

Operators as logical and types as validators:
A | B == is(A) || is(B)
ex: { a: 1 } | { b: 2 } is satisfied by { a: 1} or { b: 2 } but not { a: 1, b: 2}
A & B == is(A) && is(B)
ex: { a: 1, b: 2 } & { b: 2, c: 3} is satisfied by { a: 1, b: 2} or { b: 2, a: 3 } but not { a: 1, b: 2, c: 3}

Honestly I'd like both of these AND have types closed by default (otherwise unions are pretty silly). I'd say || should be the latter and | should be the former. Also, XOR is super nice (and use ^ or ^^ for that).

In the end (probably including things outside the scope of this bug), I'd love to be able to describe types like these:

type Omit<K, T extends Object> = { [O in (keyof T ^ K & keyof T)]: T[O] };
type Fluent<R, F extends Function> =  (...argumentsOf F) => R;
type LazyFetch<F extends Function,E> (timeout: Number) => (...argumentsOf F) => Promise<Return<F>, E>;
type IsEqual<A, B> = A extends B ? B extends A ? true : false : false;
type FilterListTypes<L, T> = L extends [infer U] ? U && T : never;
type Transition<S1, E, S2> = (S1, E) => S2
type TNextStates<T> = T extends Transition<any, any, infer U> ? U : never;
type TEvents<T> = T extends Transition<any, infer U, any> ? U : never;
// Types a function where next state is known when current state and event are given
// Also filters the possible events given an initial state
type Machine<Transitions> =
  Transitions extends [Transition<infer CurrentStates, any, infer NextStates>]
  ? (CurrentState in CurrentStates, 
      // filter events by initial state
      Event in TEvents<FilterListTypes<Transitions, Transition<CurrentState, any, any>>
    ) =>
    // filter next state by initial state and event
    TNextStates<FilterListTypes<Transitions, Transition<CurrentState, Event, any>>>
  : never // list isn't Transitions;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Issues which may not have code impact
Projects
None yet
Development

No branches or pull requests

4 participants