Improve narrowing of generic types constrained to unknown#60816
Improve narrowing of generic types constrained to unknown#60816Andarist wants to merge 4 commits intomicrosoft:mainfrom
unknown#60816Conversation
typescript-bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
| } | ||
|
|
||
| const originalType = type; | ||
| type = getNarrowableTypeForReference(type, node, checkMode); |
There was a problem hiding this comment.
there are 2 other calls to getNarrowableTypeForReference that are now not accompanied by this recombineUnknownType dance. Before landing this, I should recheck both to see if that's OK.
One of them is in checkReturnExpression - @gabritto do you have an opinion about this? This fix improves this scenario:
// this one doesn't work without this fix
function test1<T extends unknown>(
x: T,
): T extends {} ? {} : T extends Nullable ? Nullable : never {
if (x == undefined) {
return x;
} else {
return x;
}
}
type Nullable = null | undefined;
// this one works
function test2<T extends {} | Nullable>(
x: T,
): T extends {} ? {} : T extends Nullable ? Nullable : never {
if (x == undefined) {
return x;
} else {
return x;
}
}but since the returned type can't "escape" the return statement, it feels redundant to recombine that unknown type there
| // a2 is not narrowed | ||
| a2.x // error, but should be ok |
There was a problem hiding this comment.
those were outdated comments, as we can see here - no errors are reported here (rightfully so). I took the liberty to fix those here
| const y: null | undefined = x; // ok | ||
| ~ | ||
| !!! error TS2322: Type 'T' is not assignable to type 'null | undefined'. | ||
| !!! related TS2208 narrowUnknownByTypePredicate.ts:37:14: This type parameter might need an `extends null | undefined` constraint. |
There was a problem hiding this comment.
This one really shouldn't error and it should 100% behave like the other case added here. I could work on this further - but like, the simplest potential fix would be to... return unknownType from getBaseConstraintOfType when constraint === noConstraintType. I'll try to do that but I expect that there might be some reason why that isn't unified already like this despite the fact that constraint of an unconstrained type parameter is unknown (in TS files)
| x: T, | ||
| ): T extends Nullable ? Nullable : never { | ||
| if (x == undefined) { | ||
| return x; |
There was a problem hiding this comment.
in theory, this shouldn't report an error but this code is just awfully non-sensical, I only have added it as extra precautions against potential crashes (that didn't happen - but somewhat only because some helper is OK with undefined argument)
|
With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies). Next steps for PRs:
|
3 participants
fixes #60795 (comment) , cc @gabritto