Repository navigation
Narrowing discriminated unions with union discriminant properties #31404
Copy link
Copy link
Closed
Labels
Design LimitationConstraints of the existing architecture prevent this from being fixedConstraints of the existing architecture prevent this from being fixed
Description
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
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Design LimitationConstraints of the existing architecture prevent this from being fixedConstraints of the existing architecture prevent this from being fixed
Activity
jack-williams commentedon May 15, 2019
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 (hereOptionUnion) that are definitely incompatible at the point of that individual check.Knowing that
anOption.x !== trueis sufficient to categorically rule outOption1fromOptionUnion.Knowing that
anOption.x !== falseon its own is not sufficient to rule outOption2fromOptionUnion.Knowing that
anOption.x !== undefinedon its own is not sufficient to rule outOption2fromOptionUnion.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
xisneverin the else branch because it composes the two checks in the||, but the composed checks are not used for discriminant narrowing.Ideally you want to identify that any object that has a property of type
nevermust itself benever. 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:
never.j-morone commentedon Mar 10, 2020
I 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.
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 optionalisHighPriorityas a discriminant and allow the additional props iff it is present and truthy.Instead
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
StrictUnionhelper from the linked issue to get the behavior I want currently. But I would expect this as default for optional discriminantsandrewbranch commentedon Dec 1, 2020
j-morone that is a separate issue; I filed it at #41759.
[-]Using undefined as a property discriminated union doesn't work correctly with control flow narrowing[/-][+]Narrowing discriminated unions with union discriminant properties[/+]Peeja commentedon May 24, 2023
Jack Williams (@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?
exoRift commentedon Jan 30, 2025
I've devised a workaround to this issue
#61075 (comment)