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

Type guard not work in union type of objects #30506

Closed
zheeeng opened this issue Mar 20, 2019 · 5 comments
Closed

Type guard not work in union type of objects #30506

zheeeng opened this issue Mar 20, 2019 · 5 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@zheeeng
Copy link

zheeeng commented Mar 20, 2019

TypeScript Version: 3.3.3.3333

Search Terms:
type guard, union type of objects, typeof

Code

function fn(i: { foo: string, bar: number, baz: boolean } | { foo: number, bar: boolean }) {
    // Notice: i.foo has two different types in intelligence and after written down.
    if (typeof i.foo === 'string') {
       // as screenshot, in intelligence, 'i.foo' is of type 'string | number' 
        i.foo // after written down, i.foo becomes 'string'
        i.bar // always 'number | boolean'
    }
}

Code 2

function fn(i: { foo: string, bar: number, baz: boolean } | { foo: number, bar: boolean }) {
    // NOTICE: we added parenthesis
    if ((typeof i.foo) === 'string') {
        i.foo // string | number

        i.bar // number | boolean
    }
}

Expected behavior:

in 'if' branch, ‘i’ should be { foo: string, bar: number, baz: boolean }

Actual behavior:

As comments says.

image

Playground Link:
http://www.typescriptlang.org/play/index.html#src=function%20fn(i%3A%20%7B%20foo%3A%20string%2C%20bar%3A%20number%2C%20baz%3A%20boolean%20%7D%20%7C%20%7B%20foo%3A%20number%2C%20bar%3A%20boolean%20%7D)%20%7B%0D%0A%20%20%20%20if%20(typeof%20i.foo%20%3D%3D%3D%20'string')%20%7B%0D%0A%20%20%20%20%20%20%20%20i.foo%20%2F%2F%20string%20%7C%20number%0D%0A%20%20%20%20%20%20%20%20i.bar%20%2F%2F%20number%20%7C%20boolean%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A

@dragomirtitian
Copy link
Contributor

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

Might be other scenarios that I haven't quoted here, but the scenario in the code is not covered by any as far as I can tell.

As a side note, you can discriminated the union in the example using an in type guard:

function fn(i: { foo: string, bar: number, baz: boolean } | { foo: number, bar: boolean }) {
    if ('baz' in i) {
        i.foo // string
        i.bar // number
    }
}

Or use a custom type guard.

@zheeeng
Copy link
Author

zheeeng commented Mar 20, 2019

@dragomirtitian I have a question on why TS doesn't provide us the typeof guarding on codes above? Is there an underlying unsafe situation?

@dragomirtitian
Copy link
Contributor

dragomirtitian commented Mar 20, 2019

@zheeeng These cases must be specifically crafted to work, they don't just happen. I am not sure there is a definitive answer as to why more cases are not considered as property discriminants but @ahejlsberg mentions in the original discriminated union PR in a response to a similar request to yours that:

One concern is how this would affect performance. It seems that for every reference to x in a control flow graph we would now have to examine every type guard that has x as a base name in a dotted name. In other words, in order to know the type of x we'd have to look at all type guards for properties of x. That has the potential to generate a lot of work.

Maybe someone in the team can tell us if this is still the definitive reason or there are other concerns.

@jack-williams
Copy link
Collaborator

jack-williams commented Mar 20, 2019

The lack of narrowing isn't because the type of foo is not a discriminant type; a typeof type guard has never discriminated union types. Are there compelling use-cases where narrowing would be desirable? In my limited experience I've never bumped into this restriction.

@zheeeng zheeeng closed this as completed Mar 20, 2019
@sandersn sandersn added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Sep 5, 2019
@DanKaplanSES
Copy link

@jack-williams

... a typeof type guard has never discriminated union types. Are there compelling use-cases where narrowing would be desirable? In my limited experience I've never bumped into this restriction.

As a newbie, this can be confusing. For example, when I look at this stackoverflow question, the code looks like it should compile (to me).

The official documentation has a section for nearly every form of JavaScript "narrowing" I can think of. Then it goes even further, providing TypeScript-only solutions like Type Predicates.

From my perspective, the most compelling reason to implement this is to make the language feel consistent. Currently, typeof not discriminating unions feels like an arbitrary restriction that will surprise most people that run into it for the first time.

Now having said all that, I just wanted to answer your question since nobody else did. In my opinion, this might be more of a documentation issue than a problem with the language. And for that reason, I plan to open a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

5 participants