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

Nested Tagged Unions #18758

Open
ccorcos opened this issue Sep 26, 2017 · 26 comments
Open

Nested Tagged Unions #18758

ccorcos opened this issue Sep 26, 2017 · 26 comments
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript

Comments

@ccorcos
Copy link

ccorcos commented Sep 26, 2017

TypeScript Version: 2.5.2

Code

type A = { type: "a", a: number }
type B = { type: "b", b: number }

type X = { type: A, a: string }
type Y = { type: B, b: string }

let x: X | Y

if (x.type.type === "a") {
	x.a // Type Error
}

Expected behavior:

I would expect x to have type X inside the if-statement after disambiguating the nested tagged union types.

Actual behavior:

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Sep 26, 2017
@RyanCavanaugh
Copy link
Member

This isn't currently how narrowing works, but we could consider expanding its rules to cover this case

@ccorcos
Copy link
Author

ccorcos commented Sep 26, 2017

Hmm. I'm surprised. So its non-recursive?

@RyanCavanaugh
Copy link
Member

Not sure if "recursive" is the right word to apply to this situation. A discriminant property only applies to the object it's directly a member of. So in your example, inside the if block, you can access x.type.a (but not x.type.b), but there are no effects on the containing object x.

@ccorcos
Copy link
Author

ccorcos commented Sep 26, 2017

yeah, but given my type definition that seems incomplete, doesnt it?

@RyanCavanaugh
Copy link
Member

I agree

@kitsonk
Copy link
Contributor

kitsonk commented Sep 27, 2017

I have run across something similar a few times, where having to peel off/unwrap properties (DOM events come to mind) and operate/assert on different parts and pieces.

It does seem to be a similar problem space to #11117, where the value of one property effects the type of another, though obviously with the above the compiler could already deduce that without any additional syntax, which #11117 would likely require.

@pzavolinsky
Copy link

I've been doing some digging in src/compiler/checker.ts and I guess the fix should be somewhere around here:

function isMatchingReference(source: Node, target: Node): boolean {
  switch (source.kind) {
    case SyntaxKind.Identifier:
      return target.kind === SyntaxKind.Identifier && getResolvedSymbol(<Identifier>source) === getResolvedSymbol(<Identifier>target) ||
        (target.kind === SyntaxKind.VariableDeclaration || target.kind === SyntaxKind.BindingElement) &&
        getExportSymbolOfValueSymbolIfExported(getResolvedSymbol(<Identifier>source)) === getSymbolOfNode(target);
        // || (target.kind === SyntaxKind.PropertyAccessExpression && whaever else that needs to be checked here) 

Of course given the complexity of this file (createTypeChecker is a single function with ~30K lines of code), I'm not sure what the impact of this change is and what else could break from making the change.

Anyway, hope this helps, I'd really like to have this working.

Cheers!

@AnyhowStep
Copy link
Contributor

I was about to create a suggestion along the lines of "Narrow Tagged Union Recursively".

With the following examples,

A contrived example,

type T = (
    {
        success: { value: true },
        field: number
    } |
    {
        success: { value: false }
    }
);
declare const t: T;
if (t.success.value) {
    /*
        `t`` not narrowed to
        {
            success: { value: true },
            field: number
        }
    */ 
    console.log(t.field); //Error
}

Less contrived,

type T = (
    {
        payIn: { success: true, information : string },
        payOut?: { otherInformation : string }
    } |
    {
        payIn: { success: false, error : string }
    }
);
declare const t: T;
if (t.payIn.success) {
    /*
        `t` not narrowed to
        {
            payIn: { success: true, information : string },
            payOut?: { otherInformation : string }
        }

        But `t.payIn` narrowed to
        { success: true, information : string }
    */
    console.log(t.payIn.information); //OK
    console.log(t.payOut); //Error
}

@amilner42
Copy link

I currently am duplicating my tags to have them at every level.

Instead of:

export type DiffWithFiles =
  { diff: ModifiedFileDiff, previousFileContent: string, fileContent: string } |
  { diff: RenamedFileDiff, previousFileContent: string, fileContent: string, } |
  { diff: NewFileDiff } |
  { diff: DeletedFileDiff }

I have to do:

export type DiffWithFiles =
  { type: "modified", diff: ModifiedFileDiff, previousFileContent: string, fileContent: string } |
  { type: "renamed",  diff: RenamedFileDiff, previousFileContent: string, fileContent: string } |
  { type: "new", diff: NewFileDiff } |
  { type: "deleted", diff: DeletedFileDiff }

Where that type is identical to the nesteddiff.type field

I'm finding writing typescript with lots of these unions to be very pleasant, this would make that process simpler and more effective. Thanks for adding these ADT features to begin with!

amilner42 added a commit to amilner42/viva-doc that referenced this issue Feb 6, 2019
- Added content[] to VdTags for eventual propogation to the client
- Unfortunetly need repeated tags because of issue: microsoft/TypeScript#18758 so we have
  2 diffTypes
@amilner42
Copy link

I ended up squishing my types all down to the same level to avoid these issues. One issue with this approach is the field-name collisions, which makes it feel less composable, but if every field in all the objects have unique names then it's probably just as composable.

Like:

export type DiffWithFiles =
  ModifiedDiffWithFiles |
  RenamedDiffWithFiles |
  NewDiffWithFiles |
  DeletedDiffWithFiles


export type ModifiedDiffWithFiles = ModifiedFileDiff & {
  previousFileContent: string,
  fileContent: string
}

I haven't thought about the nesting enough to know all the implications.

I just wanted to post this so you see how the issue comes up in the real world and one possible way of people fixing it so you see what the current version of typescript is encouraging. There may be some negatives as well to allowing this nesting of unions, I'm not sure, I know in elm-lang they try to avoid nesting in general for various reasons. But if we don't have the nesting it seems that we need a way to deal with the name collision problem so we can & things together.

@pzavolinsky
Copy link

pzavolinsky commented Feb 7, 2019

Unfortunately in my scenario I'm typing messages whose structure is beyond my control (i.e. they are produced by a different app).

I'd really like to have support for nested discriminant props to avoid having to have some monstrosity like:

export const isMessage = <T extends Message>(
  m: Message,
  action: T['operation']['action'],
): m is T =>
  m.operation.action === action;

That is used like:

  if (M.isMessage<M.FilePreview>(m, 'FILE_PREVIEW')) {
    return filePreview(m);
  }

When this could've been as simple as:

if (m.operation.action === 'FILE_PREVIEW') {
    return filePreview(m);
}

or even:

switch(m.operation.action) {
  case 'FILE_PREVIEW': return filePreview(m);
  ...
}

@jeremybparagon
Copy link

Even if this worked only with an object spread, that would help in some unambiguously valid cases:

type A = { b: 'c' | 'd'; };
type B = { b: 'c' };
const e =
    (a: A) => {
        if (a.b === 'c') {
            const f: B = { ...a }; // Error: Type '{ b: "c" | "d"; }' is not assignable to type 'B'.
        }
    };

https://www.typescriptlang.org/play/#src=type%20A%20%3D%20%7B%20b%3A%20'c'%20%7C%20'd'%3B%20%7D%3B%0D%0Atype%20B%20%3D%20%7B%20b%3A%20'c'%20%7D%3B%0D%0Aconst%20e%20%3D%0D%0A%20%20%20%20(a%3A%20A)%20%3D%3E%20%7B%0D%0A%20%20%20%20%20%20%20%20if%20(a.b%20%3D%3D%3D%20'c')%20%7B%0D%0A%20%20%20%20%20%20%20%20%20%20%20%20const%20f%3A%20B%20%3D%20%7B%20...a%20%7D%3B%20%2F%2F%20Error%3A%20Type%20'%7B%20b%3A%20%22c%22%20%7C%20%22d%22%3B%20%7D'%20is%20not%20assignable%20to%20type%20'B'.%0D%0A%20%20%20%20%20%20%20%20%7D%0D%0A%20%20%20%20%7D%3B%0D%0A

@pelepelin
Copy link

I'd like to add another simple case which confusingly does not compile.

interface IT {
  v?: string;
}

const x: IT = {
  v: 'some'
};

let y: Required<IT>;
if (x.v) {
  y = x;
}
error TS2322: Type 'IT' is not assignable to type 'Required<IT>'.
  Types of property 'v' are incompatible.
    Type 'string | undefined' is not assignable to type 'string'.
      Type 'undefined' is not assignable to type 'string'.

21   y = x;

@haltman-at
Copy link

haltman-at commented Jul 3, 2019

Just wanted to add my voice as one more person who's just had this problem come up in my code! Just going to have to throw in a bunch of type coercions for now, it seems... (Edit: Later will come back and include a link to the particular PR of mine where I had to work around this, but it's not PR'd yet.)

@haltman-at
Copy link

For those curious, this is the PR of mine where I ran into this problem, and this is the particular file where it couldn't do the necessary inference; see specifically lines 19 and 63 as the switch statements it couldn't deal with.

@marcandrews
Copy link

I am encountering this issue with a Redux reducer with nesting switch statements 😢

@ShuiRuTian
Copy link
Contributor

Hi, folks, if you are interested in this issue, could you do me a favour? Help check the test cases added in PR #38839.

For each test file(end with .ts), it would generate .types, .symbols and errors.txt file. I think .types file is the only file need to check.

I have found some cases are not correct(pointed out similar case by TS team member), but it is easy to miss the error.

@rakeshpai
Copy link

rakeshpai commented Dec 7, 2021

+1. Just ran into this. I'm trying to type an object that's not in my control (comes from a third-party API response), which looks as follows (compiles correctly, but doesn't behave as expected):

{
  type: { name: 'foo' };
  settings: { a: boolean; b: string }
}
| {
  type: { name: 'bar' };
  settings: { c: string[] }
}
| {
  type: { name: 'baz' };
  settings: { d: number[] }
}

Would love to express this correctly.

Edit: For others who might stumble across this: I was able to achieve what I wanted with User Defined Type Guards. Needed a lot of boilerplate (thanks to copilot for helping out), but at least got the job done.

type SettingType<Name extends string, Settings> = {
  type: { name: Name }
  settings: Settings
};
type SettingFoo = SettingType<'foo', { a: boolean; b: string }>
type SettingBar = SettingType<'bar', { c: string[] }>
type SettingBaz = SettingType<'baz', { d: number[] }>
type Settings =
  | SettingFoo
  | SettingBar
  | SettingBaz;
const isSettingFoo = (s: Settings): s is SettingFoo => s.type.name === 'foo';
const isSettingBar = (s: Settings): s is SettingBar => s.type.name === 'bar'; 
const isSettingBaz = (s: Settings): s is SettingBaz => s.type.name === 'baz'; 

@pzavolinsky
Copy link

👋 currently there is a way (with some heavy type foo) to actually get this to work, I'll use @rakeshpai example, but others should work as well:

// The nested tagged union type
type X =
  | { type: { name: 'foo' }; settings: { a: boolean; b: string } }
  | { type: { name: 'bar' }; settings: { c: string[] } }
  | { type: { name: 'baz' }; settings: { d: number[] } };

// ========================================================================== //

// The type-level function that, given a name, returns the type(s) within the union for that name.
// The second argument is required to get "distributive conditional types" to work (more on this after
// this code snippet)
type GetTypeForName<TName, TX = X> = TX extends { type: { name: TName } }
  ? TX
  : never;

// Type predicate to run the narrowing in the outer object based on a nested discriminant
const is = <TName extends X['type']['name']>(
  x: X,
  name: TName,
): x is GetTypeForName<TName> => x.type.name === name;

// ========================================================================== //

// The function you actually wanted to write, all of the above is write-once boilerplate
const test = (x: X) => {
  // What you would like out of the box:
  // if (x.type.name === 'baz') console.log(x.settings.d);

  // What you can get, for now:
  if (is(x, 'baz')) console.log(x.settings.d);
};

You can read more about distributive conditional types here.

Also, I covered some of the building blocks for this solution ☝️ in this fan fiction story about typescript metaprogramming.

Hope this helps!

@Threnos
Copy link

Threnos commented Dec 24, 2022

Just adding my case to this.

type Int = {
  settings: {
    fieldRule: 'Int'
  }
  selections: {
    integer: number
  }
}

type Dec = {
  settings: {
    fieldRule: 'Dec'
  }
  selections: {
    decimal: number
  }
}

function f(val: Int | Dec) {
  if (val.settings.fieldRule === 'Int') {
    console.log(val.selections.integer) // <-- error TS2339: Property 'integer' does not exist on type '{ integer: number; } | { decimal: number; }'.   Property 'integer' does not exist on type '{ decimal: number; }'.
  }
}

@taralx
Copy link

taralx commented Jan 16, 2023

FWIW, it is possible to chain up the narrowing with some silly-looking code:

if (x.type.type === "a") {
    if (x.type === x.type) {
	x.a // No type error
    }
}

@ericbf
Copy link
Contributor

ericbf commented Mar 7, 2023

Adding my 2¢ here. Explicit type guards work fine to discriminate by a sub property. It would be great if implicit type guards did as well.

Playground

type A = {
    type: {
        name: "A"
    }
    a: number
}

type B = {
    type: {
        name: "B"
    }
    b: number
}

declare const aOrB: A | B

if (aOrB.type.name === "A") { // implicitly means that `aOrB` is `{ type: { name: "A" }}`
    console.log(aOrB.a) // Error
}

function hasTypeName<Name extends string>(a: { type: { name: string }}, name: Name): a is { type: { name: Name }} {
    return a.type.name === name
}

if (hasTypeName(aOrB, "A")) { // explicitly means that `aOrB` is `{ type: { name: "A" }}`
    console.log(aOrB.a) // Works
}

@crazyair
Copy link

crazyair commented Aug 3, 2023

Can it only support (string | number)[]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Discussion Not yet reached consensus Suggestion An idea for TypeScript
Projects
None yet