Allow filterType to consider union constraints of non-union types when determining never-ness #43763
Conversation
…n determining never-ness
typescript-bot
added
the
For Milestone Bug
src/compiler/checker.ts
Outdated
| @@ -23732,7 +23732,7 @@ namespace ts { | |||
|
|
|||
| function getNarrowedType(type: Type, candidate: Type, assumeTrue: boolean, isRelated: (source: Type, target: Type) => boolean) { | |||
| if (!assumeTrue) { | |||
| return filterType(type, t => !isRelated(t, candidate)); | |||
| return filterType(type, t => !isRelated(t, candidate), /*checkConstraints*/ true); | |||
There was a problem hiding this comment.
Do you anticipate this parameter being useful outside this one call? Why not just put the constraint-checking code in this one callback?
There was a problem hiding this comment.
Do you anticipate this parameter being useful outside this one call?
OK, so, get this: I removed this flag and made filterType unconditionally break apart constraints. We use filterType all over - surely this would make something fail in a case where, eg, we're looking for nullable types and start returning arbitrary Ts constrained to nullable types. Surely that would cause a test to fail. And yet, it did not. And this was spooky - is our coverage of the different ways we filter types really so poor? So I kept the flag, not wanting to accidentally break other usages of the filterType call that maybe did not expect the constraint decomposition when they were written, but are just poorly covered by tests.
Why not just put the constraint-checking code in this one callback?
I guess I could! It'd arguably be more consistent then, especially since it'd cause the constraint breakdown to be recursively invoked on unions whose members are type variables.
|
Is the self-build error an actual result of the change to |
|
Yep. |
Fixes #43719
Normally, in narrowing, we combine the true and false branch flow types, and that didn't appear to occur in the linked issue. In reality, it was happening exactly as specified - the false branch type was simply
neverbecause for the false branch we'd check ifSlices[SliceId][SliceKey]is a subtype ofNumClass<number>, which it obviously is not (after all, it's constrained toNumClass<number> | StrClass<string>, which is a supertype of it). The solution is, in the false case, allowingfilterTypeto break the type into it's constraint and see if any component of the constraint fits the bill when checking validity, since what we really want to know is if any part of the filtered type isn't a subtype of the type we're narrowing by.