Narrowing discriminated unions with union discriminant properties
JakeTunaley opened this issue · 5 comments
TypeScript Version: 3.5.0-dev.20190514
Search Terms: undefined discriminated union discriminant discriminator unions type guard control flow narrowing
Code
// --strictNullChecks
interface Option1 {
x: true;
}
interface Option2 {
x: false | undefined;
}
type OptionUnion = Option1 | Option2;
function acceptOption1(x: Option1): void {}
function acceptOption2(x: Option2): void {}
function acceptNever(x: never): void {}
function test(anOption: OptionUnion): void {
if (anOption.x === true) {
acceptOption1(anOption);
} else if (anOption.x === false || anOption.x === undefined) {
acceptOption2(anOption);
} else {
acceptNever(anOption);
}
}Expected behavior:
No errors.
Actual behavior:
acceptNever(anOption); has an error: Argument of type 'Option2' is not assignable to parameter of type 'never'. ts(2345)
Note that the code works correctly if the discriminator is not a property:
// --strictNullChecks
type Option1 = true;
type Option2 = false | undefined;
type OptionUnion = Option1 | Option2;
function acceptOption1(x: Option1): void {}
function acceptOption2(x: Option2): void {}
function acceptNever(x: never): void {}
function test(anOption: OptionUnion): void {
if (anOption === true) {
acceptOption1(anOption);
} else if (anOption === false || anOption === undefined) {
acceptOption2(anOption);
} else {
acceptNever(anOption); // Not an error - correctly narrows to `never`
}
}Playground Link:
Related Issues:
#14471 is also using undefined as a discriminant, although the linked issue is restricting itself to talking about mapped types
I think this is a design limitation in discriminant narrowing which is effectively a top-down process, rather than bottom up.
When narrowing for the negation of a discriminant check - so when something like anOption.x === ${THING} is assumed to be false - the checker will try and filter out branches in the declared union (here OptionUnion) that are definitely incompatible at the point of that individual check.
Knowing that anOption.x !== true is sufficient to categorically rule out Option1 from OptionUnion.
Knowing that anOption.x !== false on its own is not sufficient to rule out Option2 from OptionUnion.
Knowing that anOption.x !== undefined on its own is not sufficient to rule out Option2 from OptionUnion.
Only using the composition of the two is it possible to rule out Option2, however the checker will not compose multiple property narrowings when discriminant pruning. So by top-down I mean that it will not collect state from composite narrowings of the same property and use them.
For example, the checker knows that x is never in the else branch because it composes the two checks in the ||, but the composed checks are not used for discriminant narrowing.
function test(anOption: OptionUnion): void {
if (anOption.x === true) {
acceptOption1(anOption);
} else if (anOption.x === false || anOption.x === undefined) {
acceptOption2(anOption);
} else {
anOption.x // x is never
acceptNever(anOption);
}
}Ideally you want to identify that any object that has a property of type never must itself be never. Though, as I've learned the hard way, it is easy to introduce massive performance regressions by changing discriminant narrowing.
There may be some changes that could be done in the checker to support this; someone on the team would need to look into it. Two workarounds for now:
- Separate out the union into three parts.
interface Option1 { x: true; }
interface Option2 { x: false; }
interface Option3 { x: undefined; }
type OptionUnion = Option1 | Option2 | Option3;- Use the property itself as proof of
never.
acceptNever(anOption.x); // okI have a case that may be more related to #20863 from a technical standpoint, but since it is specific to undefined, I thought I'd add here.
I'd like to have an interface that conditionally has a set of additional optional properties based on the truthiness of an optional property.
interface CommonProps {
a: number;
b: string;
c: boolean
}
interface HighPriorityProps {
neededBy?: DateTime;
priorityLevel?: number;
}
interface HighPriorityIssueProps extends HighPriorityProps{
isHighPriority: true
}
interface RegularPriorityIssueProps {
isHighPriority?: false
}
type Props = CommonProps & (HighPriorityIssueProps | RegularPriorityIssueProps)
The use case is for a React component where for ~80% of the use cases I don't want to have to pass in isHighPriority={false}. I'd like to be able to use the optional isHighPriority as a discriminant and allow the additional props iff it is present and truthy.
Instead
<Issue priorityLevel={1}/>
does not throw an error, when I expect it to. I can not use the first workaround, because then I would manually have to still include the optional prop as <Issue isHighPriority={undefined} > instead of excluding it, which is what I want users of the API to be able to do. I cannot use the second as this is in a React component and not a function.
Let me know if I should open a separate issue. Since this is close to many open issues, I didn't want to add another to the list!
EDIT: By the way, I am using the StrictUnion helper from the linked issue to get the behavior I want currently. But I would expect this as default for optional discriminants
@jack-williams I'm not sure I understand why this is a design limitation. The checker does know how to narrow the discriminant property itself. Couldn't the object's type be narrowed at the same time?
I've devised a workaround to this issue