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

Union not narrowed with typeof x.y as a discriminant #32399

Open
tusklozeleniy opened this issue Jul 15, 2019 · 11 comments
Open

Union not narrowed with typeof x.y as a discriminant #32399

tusklozeleniy opened this issue Jul 15, 2019 · 11 comments
Labels
Bug A bug in TypeScript Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this
Milestone

Comments

@tusklozeleniy
Copy link

TypeScript Version: 3.6.0-dev.20190713

Search Terms: TS2339, union type object, union type property does not exist
And github suggestions while I was writing the title.

Code

type Test = {
    someKey: false
} | {
    someKey: {
        someKeyInside?: number
    },
    someKeyThatExistOnlyIfOtherKeyIsNotFalse: string;
};

declare const test: Test;

if (typeof test.someKey === 'object') {
    console.log(test.someKeyThatExistOnlyIfOtherKeyIsNotFalse);
}

Expected behavior:
String in someKeyThatExistOnlyIfOtherKeyIsNotFalse should be logged

Actual behavior:

error TS2339: Property 'someKeyThatExistOnlyIfSomeKeyIsNotFalse' does not exist on type 'Test2'.
  Property 'someKeyThatExistOnlyIfSomeKeyIsNotFalse' does not exist on type '{ someKey: false; }'.

29     console.log(test2.someKeyThatExistOnlyIfSomeKeyIsNotFalse); // TS2339: Property 'someKeyThatExistOnlyIfSomeKeyIsNotFalse' does not exist on type '{ someKey: false; }'.

Playground Link:
https://www.typescriptlang.org/play/index.html#code/C4TwDgpgBAKhDOwCMUC8UDeAoKurwHsBbCAaQhAC4oAzAQwBt4IsBfKAH0xz0JPKrc8w-MTIUAkgDt4ASwAmEalICuRAEYQATj1ysANLtH8KMABZ1gAUQAesxAHkpDEBJoPgZ7QInwAcgTAAGKMzNSIWrJSAOYA3GzxWIoAxgx0WtDJBDLAUMAIyNRwiEiJsjRQABT5JQB0fOIgAJRCeFkyBAwQtQwE0dUFSPViAuaWtvbATi5uHl5aPv6BIUwQTbFQAPSbUA6kAIRsWFigkLAFAExorbgNAtT0q2ycN8aN1NgieNuwXlDZLn+Umg8nKNG0ECkyWgNC0xDy4GgxWQ+jePhkCkydCkUE0UBUUkUNCiEHkQLRFCMwjukgxigA-Mo1JodMIDEYaSAxtY7I5nK53J5vJIlsFQkp8MBIjF4qxEik0hkoO1EHlLkVLmUKtVEQQKjVgBdhiYQGhUOgAOQEdQAKwgyWAFpanza2UIXR6fQGiCNnO5Ez5M0F80WATFq3WWx2MAAyhcAMzxgCc1AACnDIFpQFALX6LDzJtMBXNha5RStmBaoPICAgoFJAlAILzctkEWcLRgKYJHswNqwLbU2EA

Take a look at type Test1. The only one difference from type Test2 is someKeyInside should be always defined.

Related Issues:

@RyanCavanaugh RyanCavanaugh changed the title TS2339 Union type object bug Union not narrowed by typeof x.y === 'object' with mixed primitive and nonprimitive discriminants Jul 15, 2019
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jul 15, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jul 15, 2019
@jack-williams
Copy link
Collaborator

I'm not sure typeof has ever triggered narrowing by discriminant.

@bterlson
Copy link
Member

Just chiming in to say I would love this fixed! Our repro is something like the following:

interface A { x: string, y: string };
interface B { x: number, y: number };
type X = A | B;
 
declare var bar: X;
 
if (typeof bar.x === 'string') {
    let y = bar.y; // string | number, but really should be string.
}

Essentially we have some methods we'd like to return union of object types from. The object types are mostly distinct with a little overlap. Ideally users would just check the field that they want is present, and TS would do the math and present a proper type inside the if block.

We ran in to this because we were using a tagged union pattern which was working fine, but then attempted to generalize it, and found the narrowing stopped working.

@DanielRosenwasser DanielRosenwasser changed the title Union not narrowed by typeof x.y === 'object' with mixed primitive and nonprimitive discriminants Union not narrowed with typeof x.y as a discriminant Feb 1, 2020
@DanielRosenwasser DanielRosenwasser added Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this labels Feb 28, 2020
@ShuiRuTian
Copy link
Contributor

Looks interesting. I would have a try on this.

@ShuiRuTian
Copy link
Contributor

Hi, if I define

interface Foo1 {
    key: number|undefined;
    f1: number;
}

interface Foo2 {
    key: string|undefined;
    f2: number;
}

type U = Foo1 | Foo2

function f1(u: U) {
    if (typeof u.key !== 'number' && typeof u.key !== 'undefined') {
        u;       // What is this?
        u.key; // What is this? 
    }
}

What type of u and u.key should be?
Should it be Foo2? If so, could some guy give me some advice about how to change the flow?

@jack-williams
Copy link
Collaborator

Maybe it should have comparable behavior to the literal type case?

interface Foo1 {
    key: "number" | "undefined";
    f1: number;
}

interface Foo2 {
    key: "string" | "undefined";
    f2: number;
}

type U = Foo1 | Foo2

function f1(u: U) {
    if (u.key !== 'number' && u.key !== 'undefined') {
        u;       // U
        u.key;   // 'string'
    }
}

@danvk
Copy link
Contributor

danvk commented Jun 5, 2020

I ran across this after seeing @jack-williams's comment here: #30622 (comment)

I was trying to "entangle" the refinements on two types by sticking them in an object together and was surprised that it didn't work:

function f(x: string | number, y: string | number) {
  let o;
  if (typeof x === 'string' && typeof y === 'string') {
    o = { x, y };
  } else if (typeof x === 'number' && typeof y === 'number') {
    o = { x, y };
  } else {
    throw new Error(`Can't have mixed types: ${typeof x} and ${typeof y}`);
  }

  o;  // type is { x: string; y: string; } |
      //         { x: number; y: number; }
  o.x;  // type is string | number
  if (typeof o.x === 'string') {
    o.x;  // type is string
    o;  // type is the same as above
    o.y;  // type is string | number :(
  }
}

(playground)

@lazytype
Copy link

This would be useful to have since the improved tuple types in 4.0 will make overload implementations that use tuples even more common.

function Example(a: number, b?: number): number;
function Example(a: string, b?: string): string;
function Example(...args: [a: number, b?: number] | [a: string, b?: string]): number | string {
  if (typeof args[0] === 'number') {
    // doesn't narrow
  } else {
    // doesn't narrow
  }
}

@octogonz
Copy link

Is this a duplicate of #30506?

For its case, #30506 (comment) explained that:

The type of foo inside the if is string so the type guard does work on that field as it should. The code completion entry does not show the flow type (might be the topic of a different issue), but if you try to . into foo you will see it is a string.

Narrowing the parent object is only done in specific scenarios, when the property is considered a discriminant for the union. A property is considered as a discriminant property if:

  1. The property is a literal type as outlined here Discriminated union types #9163
  2. The a property of a union type to be a discriminant property if it has a union type containing at least one unit type and no instantiable types as outlined here Allow non-unit types in union discriminants #27695

Although this issue involves typeof x.y, the principle seems pretty much the same. (?)

@xirzec
Copy link
Member

xirzec commented Aug 26, 2021

The linked meeting notes about this (#36877) seem to suggest there is a possible solution (feature request):

discriminate inside of typeof test (make typeof apply discriminate narrowing)

Hopefully this is still feasible, and we don't have to close this issue as a "design limitation" like in #30506?

@danvk
Copy link
Contributor

danvk commented Jan 24, 2024

Pulling in Ryan's comment from 2022 as the latest update on this issue #38839 (comment):

From the looks of it, this feature is unfortunately much more complex to implement than we had anticipated, and we don't think that the cost/benefit ratio is good in this case. As much as we want to support this pattern, the implications of these changes feel too far-reaching (even if they're necessary to actually support the scenario). We'll leave the original issue open in case later on down the line a simpler method of approach becomes available, but we aren't comfortable with introducing this much complexity in a critical codepath right now.

All that said, putting this much time into the investigation is something we appreciate a ton. Thank you for all the effort you've put in into authoring this.

@jakebailey
Copy link
Member

This was fixed in #50344, as far as I can tell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Effort: Moderate Requires experience with the TypeScript codebase, but feasible. Harder than "Effort: Casual". Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.