-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
discriminated union type matching behaviour changed starting from 5.2.x #57231
Comments
I have bisected this to #53907 . It means that this code with this change should roughly be equivalent to the one with type Food = 'apple' | 'orange';
type Vegetable = 'spinach' | 'carrot';
type Other = 'milk' | 'water';
type Custom = 'air' | 'soil';
type Target =
| {
audience: 'earth';
meal:
| Custom
| `fruit_${Food}`
| `vegetable_${Vegetable}`
| `other_${Other}`;
}
| {
audience: 'mars' | 'jupiter';
meal: string;
}
const target: Target = {
audience: 'earth',
meal: `vegetable_carrot`
};
const target2: Target = {
meal: `vegetable_carrot`,
audience: 'earth'
}; This one doesn't error though. I'll investigate further what's the difference and what the behavior should be. |
What, no tomatoes? ...oh no, they didn't turn carnivorous and go on a rampage did they? because if so... run |
This is fun π type Food = "apple" | "orange";
type Vegetable = "spinach" | "carrot";
type Other = "milk" | "water";
type Custom = "air" | "soil";
type Target =
| {
audience: "earth";
meal:
| Custom
| `fruit_${Food}`
| `vegetable_${Vegetable}`
| `other_${Other}`;
}
| {
audience: "mars" | "jupiter";
meal: string;
};
const vegetable1: Vegetable = "carrot";
// it errors but it shouldn't
const target1: Target = {
meal: `vegetable_${vegetable1}`,
audience: "earth",
};
const vegetable2: "carrot" = "carrot";
// it errors but it shouldn't
const target2: Target = {
meal: `vegetable_${vegetable2}`,
audience: "earth",
};
const vegetable3 = "carrot";
// ok
const target3: Target = {
meal: `vegetable_${vegetable3}`,
audience: "earth",
}; I'll push out a fix for this soon. |
A simpler repro that isn't fixed by #57236: type Vegetable = 'spinach' | 'carrot';
type Target =
| { audience: 'earth', meal: `vegetable_${Vegetable}` }
| { audience: 'mars', meal: string };
declare const vegetable: Vegetable;
// Ok
const target: Target = {
audience: 'earth',
meal: `vegetable_${vegetable}`
};
// Errors
const target2: Target = {
meal: `vegetable_${vegetable}`,
audience: 'earth'
}; The root problem is that contextual types are narrowed by discriminant properties in the order those discriminant properties are written (as opposed to some canonical narrowing that considers all of them at the same time). Above, the assignment to In reality, |
Given how often people want to be able to write type DU =
| { type: "foo", foo: string }
| { type: "bar", bar: string }
| { type: string, catchall?: string } which would likely be broken by |
There probably is, but it is hard to see what it accomplishes. The only property that can be made accessible through narrowing is |
π Search Terms
"key order when matching object types" "discriminated unions key order"
π Version & Regression Information
β― Playground Link
https://www.typescriptlang.org/play?ts=5.2.2#code/FAFwngDgpgBAYgewQExgXhgcgIYQgGykxgB8sEAnbAOwHMiBuUSWANSnpGwCND0sAzhACW1bAGMAFsTKZx2ChQQhMTcNBgB5EJKgV+mALbD8AaxlYA7thB7VzDQGEArgJAJDB7MIoXMAhBN7B1gYABUFTnRgGFi4sgBvGLiU2OxnZGEoanEoAC4sKAUde1TUwyL8POSylLIXNw8a2tiyAAMAMwpnYRAAfQASBMQUAF825pb2gDcOKC5eKEGE9k4eQnHJ2vblXQpl7T3xphbRrcStlPTM7NyCowUBPwArZxFbXxOWmArsKpg3BRRLQvmUzsBgOIENQ3DBZmtFgVVvN1rAMHIFEoVExIdDYVwKJwChFCfN+DAklcMlkcvlCsVpAAaZq-f5teEoxbLDkLDYTUY4qEwkAwAmcABMxMiZIwFJZlQK7LmvKWQx5qPGzKpN1p9yKFBKwAFwCAA
π» Code
Output
Compiler Options
Playground Link: Provided
π Actual behavior
In the code both
target
andtarget2
have the correct structure based on theTarget
type.But the order of keys is reversed in
target2
which used to work but not anymore.π Expected behavior
Both
target
andtarget2
should be valid.Additional information about the issue
Discriminated unions didn't rely on the key ordering AFAIK.
The text was updated successfully, but these errors were encountered: