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 types (Foo | Bar) inconsistently becomes Partial<Foo & Bar> #59819

Closed
davidbarratt opened this issue Sep 1, 2024 · 13 comments
Closed

Union types (Foo | Bar) inconsistently becomes Partial<Foo & Bar> #59819

davidbarratt opened this issue Sep 1, 2024 · 13 comments

Comments

@davidbarratt
Copy link

davidbarratt commented Sep 1, 2024

πŸ”Ž Search Terms

"union type partial", "type union narrowing"

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about Unions

⏯ Playground Link

https://www.typescriptlang.org/play/?exactOptionalPropertyTypes=true&ts=5.5.4#code/JYOwLgpgTgZghgYwgAgGIHt3IN4ChkHIyYBcyAzmFKAOYDcuAvrrqJLIigEJxQ76EARrzIgArgFtB0BoWQAvOPLKVqIekxYwxIBGGDoQyYfIAUAByjpz5MhiwAfZDz4BKfnOAxkpgETCoX2RQZEtrcnc8OTkEQ3J0ABsIADoE9BoLKxtkxXlXWUJGZAgE8hQo6IJYkHik1PTM8OTidHyBAmZmXBNTbCJSZF9ISiDGfOQAegnkYbBupV7jEWQARgAmAGYAFgAaBSUyXwBPMQALUfGp5BPTlivMGwBCa-QxZAQ4I3M4cnJkOAoYkEZTAyHQ3n86DA52QUDg0OgM1On0GhhQ6D4CLBCMCyRYPT6LUOs18ewCZHWG2QYzok2mOgAJhAYKAIAz5mZCQMhhARntcocGeQYAyYCsLrSrrMgA

πŸ’» Code

interface Foo {
    foo: string;
}

interface Bar {
    bar: number;
    zaz: string;
}

function baz(props: Foo | Bar ) {
    if ("bar" in props) {
        console.log(props.zaz);
    } else {
        console.log(props.foo);
    }
}

baz({ foo: "test" }); // test
baz({ bar: 1234, zaz: "yuh" }); // yuh

// oops! you can pass a subset of "both" rather than "one or the other".

baz({ foo: "test", bar: 123 }); // undefined
baz({ foo: "test", zaz: "dsfdf1" }); // test

πŸ™ Actual behavior

I was surprised that I would pass in an object that is neither Foo nor Bar and TypeScript didn't complain about. This caused a bug at runtime that it seems like TypeScript should have caught?

Interestingly, the function definition is well aware that only one or the other could be passed. I could not write a scenario where that wasn't the case, but calling the funciton is where this fails.

πŸ™‚ Expected behavior

I expected to be able to pass Foo or Bar but not both and certainly not neither.

Additional information about the issue

I couldn't find a previous version of TypeScript where this worked as I would expect it too. I was really wondering if I was doing something wrong or this was a well known issue, but I was struggling to find anything on the topic.

It looks like this also fails:

const data: Foo|Bar = {
    foo: "test",
    bar: 123,
};

in which case data is neither Foo nor Bar but TypeScript doesn't complain about it.

You can see this with both of these that fail:

const data: Foo = {
    foo: "test",
    bar: 123,
};
const data: Bar = {
    foo: "test",
    bar: 123,
};

I would think that if they fail seperately, a union should also fail?

@davidbarratt davidbarratt changed the title Union types (Foo | Bar) inconsistantly become Partial<Foo & Bar> Union types (Foo | Bar) inconsistently becomes Partial<Foo & Bar> Sep 1, 2024
@jcalz
Copy link
Contributor

jcalz commented Sep 1, 2024

TS is working as designed and you've run into a known inconsistency/unsoundness in the language.

#12936: object types in TS are not "exact" (e.g., the objects you don't like are both valid Foo objects)
#20863: excess property checks don't occur for non-discriminated unions
#34975: the in operator is intentionally unsound

@MartinJohns
Copy link
Contributor

@davidbarratt
Copy link
Author

davidbarratt commented Sep 2, 2024

@MartinJohns

If I'm understanding that correctly in these two failing cases:

baz({ foo: "test", bar: 123 }); // undefined
baz({ foo: "test", zaz: "dsfdf1" }); // test

both objects are Foo with a bar and zaz excess property respectively?

In that case, isn't it the narrowing that cannot be trusted since it shouldn't allow you to narrow it down to Bar in the function declaration since it could be Foo with an excess property?

In which case the only way to narrow it would be to eliminate Foo?

interface Foo {
    foo: string;
    jaz: number;
}

interface Bar {
    bar: number;
    zaz: string;
}

function baz(props: Foo | Bar ) {
    if (!("foo" in props)) {
        console.log(props.zaz);
    }
    else {
        console.log(props.jaz);
    }
}

baz({ bar: 123, jaz: 1234, foo: "blah" }); // 1234

Ideally, I'd like user to specify one object or the other when calling the function and have it fail if it doesn't have all the properties of one of the objects. Is that even possible?

@MartinJohns
Copy link
Contributor

both objects are Foo with a bar and zaz excess property respectively?

That's correct. And excess property checks don't kick in as per #20863 (linked by jcalz already).

In that case, isn't it the narrowing that cannot be trusted since it shouldn't allow you to narrow it down to Bar in the function declaration since it could be Foo with an excess property?

jcalz already linked the relevant issue here too: #34975. The in operator is intentionally unsound. You either accept this, or check the entire structure of your object, or (IMO the best option) add a discriminator property to your types.

Ideally, I'd like user to specify one object or the other when calling the function and have it fail if it doesn't have all the properties of one of the objects. Is that even possible?

That's... how it works with the code you have already.

@davidbarratt
Copy link
Author

I think I found a solution:

interface Foo {
    foo: string;
    zaz?: never;
    bar?: never;
}

interface Bar {
    foo?: never;
    bar: number;
    zaz: string;
}

function baz(props: Foo | Bar ) {
    if ("bar" in props) {
        console.log(props.zaz);
    } else {
        console.log(props.foo);
    }
}

baz({ foo: "test" }); // test
baz({ bar: 1234, zaz: "yuh" }); // yuh

// oops! you can pass a subset of "both" rather than "one or the other".

baz({ foo: "test", bar: 123 }); //  FAILS
baz({ foo: "test", zaz: "dsfdf1" }); // FAILS

https://www.typescriptlang.org/play/?exactOptionalPropertyTypes=true&ts=5.5.4&ssl=27&ssc=45&pln=1&pc=1#code/JYOwLgpgTgZghgYwgAgGIHt3IN4ChkHIyYBcyAzmFKAOYDc+hAXnEwPxkgQBu0DhyAEZwoHZF15QGAX1y5QkWIhQAhETkYFi6MRL6ahIzgFcAtoP0CWTMpWoh6uWbhjGQCMMHQhDTABQADlDoAeRkGFgAPshqUMgAlBoCwDDIfgBEwlDpyKDIQSHkiXgCAgje5OgANhAAdFXoNIHBobXW8fyE0sgQVeQoJaUE5SCVNfWNzYW12h0Gss7C-thEpMjpkJQ50h3IAPR7yJtguEt+K1lkAIwATADMACwANMjWZOkAnsYAFtu7B8gvt85ADMKEAISA9DGZAIOA+AJwcjkZBwCjGQT9MDIdCpTLoMC-ZBQOCE6BHb7w9beFDoOJknFk7K1ORnFbad7HdIvS7IW53ZA7Oj7Q5uAAmEBgoAgYtOrHOq3QnIgWxeb3WYvIMDFMCuf2FAOOQA

It might be helpful if the docs were updated with something like that. Like you can make your unions safer by specifying all the known properties of the other values.

@jcalz
Copy link
Contributor

jcalz commented Sep 2, 2024

#12936 (comment)

@MartinJohns
Copy link
Contributor

I wouldn't call this a "solution", but rather a crude workaround / hack. This call is still legit and will blow up:

baz({ foo: "test", get bar(): never { throw 1 } , get zaz(): never { throw 1 } });

A property typed never does not mean the property doesn't exist..

@davidbarratt
Copy link
Author

@MartinJohns Is there an issue for having a syntax for "property does not exist" ? That seems like less of a lift than #12936 right?

@MartinJohns
Copy link
Contributor

There is #55143.

@jcalz
Copy link
Contributor

jcalz commented Sep 2, 2024

@MartinJohns I think we've had this disagreement before about things like {bar?: never}. If the possibility of a getter that throws is something to worry about then you always have to worry about it because every type T is equivalent to T | never. I would just check typeof props.bar !== "undefined" (instead of using in, which I agree isn't the right check) and if it blows up there then I'm fine, right?

@davidbarratt
Copy link
Author

FWIW I find it rather unintuitive that Excess Property Checks occur, but not if the property just so happens to be part of another object in the union.

Take for instance this execution:

baz({ foo: "test", bar: 123 }); // undefined
baz({ foo: "test", saz: "test" }); // FAIL

https://www.typescriptlang.org/play/?exactOptionalPropertyTypes=true&ts=5.5.4#code/JYOwLgpgTgZghgYwgAgGIHt3IN4ChkHIyYBcyAzmFKAOYDcuAvrrqJLIigEJxQ76EARrzIgArgFtB0BoWQAvOPLKVqIekxYwxIBGGDoQyYfIAUAByjpz5MhiwAfZDz4BKfnOAxkpgETCoX2RQZEtrcnc8OTkEQ3J0ABsIADoE9BoLKxtkxXlXWUJGZAgE8hQo6IJYkHik1PTM8OTidHyBAmZmXBNTbCJSZF9ISiDGfOQAegnkYbBupV7jEWQARgAmAGYAFgAaBSUyXwBPMQALUfGp5BPTliuAYXQoKAhwBKO0OGBS+bM+lsOs18e3IB1Wm2QYzok2mqAAggBJAAyAGU7tNMDYAITXdBiZAIOBGcxwcjkZBwChiQRlMDIdDefzoMDnZBQOAs6AzU5EwaGFBPbkCzmBZIsHr-AZDCAjPYBMjrDaQy7THQAEwgMFAEDVv0WAMGQL2uUOavIMDVMBWF2hV1mQA

Why is it that excess checks just get disabled if the property happens to be bar? Clearly it's an excess property of Foo and TypeScript knows that and does it correctly when it's a different property name.

@davidbarratt davidbarratt reopened this Sep 2, 2024
@jcalz
Copy link
Contributor

jcalz commented Sep 2, 2024

looks meaningfully at #20863

@davidbarratt
Copy link
Author

@jcalz I see, this is the same issue. My apologies, I was tripped up by some of the language used in the issue. Thank you for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants