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

Unable to discriminate union type because of "Property does not exist" #28138

Closed
lemoinem opened this issue Oct 25, 2018 · 8 comments
Closed
Labels
Question An issue which isn't directly actionable in code

Comments

@lemoinem
Copy link

lemoinem commented Oct 25, 2018

TypeScript Version: typescript@3.2.0-dev.20181025

Search Terms: union narrowing discriminate missing property field

Code

class Foo
{
    private foo: string;
}

class Bar
{
    private bar: number;
}

function fn(baz: { foo: Foo; bar: string; } | { foo: Bar; })
{
    if (typeof baz.bar != 'undefined')
    {
        testFoo(baz.foo);
    }
}

function testFoo(foo: Foo)
{
}

Expected behavior:
No Error. I can call testFoo(baz.foo) since the union type is properly narrowed.

Actual behavior:
Property 'bar' does not exist on type 'Baz'.

Argument of type 'Foo | Bar' is not assignable to parameter of type 'Foo'.
Type 'Bar' is not assignable to type 'Foo'.
Property 'foo' is missing in type 'Bar'.

Playground Link: https://www.typescriptlang.org/play/index.html#src=class%20Foo%0D%0A%7B%0D%0A%20%20%20%20private%20foo%3A%20string%3B%0D%0A%7D%0D%0A%0D%0Aclass%20Bar%0D%0A%7B%0D%0A%20%20%20%20private%20bar%3A%20number%3B%0D%0A%7D%0D%0A%0D%0Afunction%20fn(baz%3A%20%7B%20foo%3A%20Foo%3B%20bar%3A%20string%3B%20%7D%20%7C%20%7B%20foo%3A%20Bar%3B%20%7D)%0D%0A%7B%0D%0A%20%20%20%20if%20(typeof%20baz.bar%20!%3D%20'undefined')%0D%0A%20%20%20%20%7B%0D%0A%20%20%20%20%20%20%20%20testFoo(baz.foo)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Afunction%20testFoo(foo%3A%20Foo)%0D%0A%7B%0D%0A%7D

Related Issues: I've looked around but found none.

Although, in general, I do understand this should not work, I think it would help greatly if we had a way to discriminate on properties that are only in some of the members of a union.

One of the solutions could be to type properties, that are missing in some of the union members as prop?: void, or prop?: never, or prop?: undefined, or whatever makes sense in the type system.

@jack-williams
Copy link
Collaborator

Although, in general, I do understand this should not work, I think it would help greatly if we had a way to discriminate on properties that are only in some of the members of a union.

This should work

function fn(baz: { foo: Foo; bar: string; } | { foo: Bar; })
{
    if ("bar" in baz)
    {
        testFoo(baz.foo);
    }
}

With this PR #27695 you can also do this:

function fn(baz: { foo: Foo; bar: string; } | { foo: Bar; bar: undefined })
{
    if (baz.bar !== 'undefined')
    {
        testFoo(baz.foo);
    }
}

@lemoinem
Copy link
Author

@jack-williams Your suggestion works. Thank you!

I'll keep this issue open as I think it should be documented and/or we could have better syntax for it... but I might be wrong. I'd like a member of TS team to have a look and provide a definitive decision on that if you don't mind...

@weswigham
Copy link
Member

Yep, with what we have in master now, you should just need to actually include the member you want to discriminate on as an (optional) undefined value for all union members it otherwise wouldn't exist on.

@weswigham weswigham added the Question An issue which isn't directly actionable in code label Oct 26, 2018
@weswigham
Copy link
Member

BTW, there's a good reason this:

function fn(baz: { foo: Foo; bar: string; } | { foo: Bar; })
{
    if (typeof baz.bar != 'undefined')
    {
        testFoo(baz.foo);
    }
}

doesn't work - consider this call:

fn({bar: 12, foo: new Bar()});

that's a valid call (assignable to the second union member), and is why even if you were to check that bar wasn't undefined in the input, you might still not have a Foo for foo. The in operator is intentionally a little unsound here, but we figured that its usage was infrequent enough that if you use it you really wanted it to act like this.

@lemoinem
Copy link
Author

Indeed. I haven't thought of this case.
I guess it would impossible to implement it in a completely sound way.

Thanks for your answer!

abrgr added a commit to pluginsdotdev/pluginsdotdev that referenced this issue Sep 15, 2020
…fering properties

microsoft/TypeScript#28138 - need to explicitly mark properties as
optional and undefined if they only appear on a subset of a union's possibilities
abrgr added a commit to pluginsdotdev/pluginsdotdev that referenced this issue Oct 2, 2020
…fering properties

microsoft/TypeScript#28138 - need to explicitly mark properties as
optional and undefined if they only appear on a subset of a union's possibilities
@nuts-n-bits
Copy link

nuts-n-bits commented Jan 14, 2021

BTW, there's a good reason this:

function fn(baz: { foo: Foo; bar: string; } | { foo: Bar; })
{
    if (typeof baz.bar != 'undefined')
    {
        testFoo(baz.foo);
    }
}

doesn't work - consider this call:

fn({bar: 12, foo: new Bar()});

that's a valid call (assignable to the second union member), and is why even if you were to check that bar wasn't undefined in the input, you might still not have a Foo for foo. The in operator is intentionally a little unsound here, but we figured that its usage was infrequent enough that if you use it you really wanted it to act like this.

Hi, I strolled by this thread randomly and I am immediately baffled as to why fn({bar:12, foo: new Bar()}) should be a valid call?
Let me play the role of a verbose compiler:

  1. value {bar: 12, foo: new Bar()} cannot be assigned to {foo: Foo, bar: string}, because bar is 12, 12 is not string.
  2. value {bar: 12, foo: new Bar()} cannot be assigned to {foo: Bar} either, because it contains an extra bit bar: 12, and there's a TS rule that says: "object literal may only specify known properties". field bar is unknown to type {foo: Bar}.
  3. So I will throw an error, I can either reject it because 12 is not a string, or I can reject it because of the extra field.

And I tested it, indeed fn({bar:12, foo: new Bar()}) is not valid call, as tsc complains that number 12 is not a string. (tested for strictNullCheck = true and false)

But then here comes the weird part: fn({bar:"lol", foo: new Bar()}) is a valid call! This surprises me, as it still doesn't fit either type, for the same reasons the verbose compiler rejected it earlier. Let me role play again:

  1. value {bar:"lol", foo: new Bar()} cannot be assigned to {foo: Foo, bar: string}, because foo is new Bar(), which is not a Foo.
  2. value {bar:"lol", foo: new Bar()} cannot be assigned to {foo: Bar} either, because it contains an extra field bar: 12, and there is a TS rule against that.
  3. I should raise an error saying either new Bar() cannot be assigned to Foo, or that there is an extra field.

But since reality doesn't agree with me, it must be I'm understanding union types wrong, or maybe TS has a bug.

So if I were to guess, it seems that TS is pulling the foo:Bar bit from the RHS of the union type, and bar:string bit from the LHS of the union type, and combining them together. If this is the expected behaviour, then, well, I learned something today, though I'm not sure I feel good about this design choice. Do you know what's happening here?

P.S. there is more craziness.

now try declare function fn(baz: {foo: null, bar: string} | { foo: bigint}) with the call fn({bar: "lol", foo: 3n}), now tsc starts complaining again, saying the extra bar field is preventing it from assigning it to {foo:bigint} (which I agree). So I cannot come up with any good reason for this inconsistency between these cases, and I'm suspecting a bug again. Please someone with more knowledge explain this to me.

@RyanCavanaugh
Copy link
Member

value {bar:"lol", foo: new Bar()} cannot be assigned to {foo: Bar} either, because it contains an extra field bar: 12, and there is a TS rule against that.

This is the wrong bit -- extra fields are allowed when there is some matching property in the target across any union member.

@nuts-n-bits
Copy link

nuts-n-bits commented Jan 14, 2021

This is the wrong bit -- extra fields are allowed when there is some matching property in the target across any union member.

Yeah, about that.

That was my initial feeling so I did some more testing which ended up contradicting this.

declare function id_or_name(input: {id:number; name:"joe"}|{name:string}): void
id_or_name({id:3, name:"alex"})  // Argument of type '{ id: number; name: string; }' is not assignable to parameter of type '{ id: number; name: "joe"; } | { name: string; }'. Object literal may only specify known properties, and 'id' does not exist in type '{ name: string; }'.ts(2345)

Somehow this time it doesn't work again. I notice though that if instead of having name:<primitive type/literal type> in the parameter, you have name: <Class type>, then it behaves like you describe again.

declare function id_or_name(input: {id:number; name:Date}|{name:string}): void
id_or_name({id:3, name:"alex"})  // no problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

5 participants