-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Use comparability for discriminant properties when narrowing types for a default switch clause #61211
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
Use comparability for discriminant properties when narrowing types for a default switch clause #61211
Conversation
…r a default switch clause
| return caseType; | ||
| } | ||
| const defaultType = filterType(type, t => !(isUnitLikeType(t) && contains(switchTypes, t.flags & TypeFlags.Undefined ? undefinedType : getRegularTypeOfLiteralType(extractUnitType(t))))); | ||
| const defaultType = filterType(type, t => !(isUnitLikeType(t) && contains(switchTypes, t.flags & TypeFlags.Undefined ? undefinedType : getRegularTypeOfLiteralType(extractUnitType(t)), (t1, t2) => isUnitType(t1) && areTypesComparable(t1, t2)))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This essentially matches the logic used by narrowTypeByEquality here
if (isUnitType(valueType)) {
return filterType(type, t => !(isUnitLikeType(t) && areTypesComparable(t, valueType)));
}As we can see, the valueType has to pass isUnitType there and the equivalent of the valueType here are all the types in switchTypes. Note that in the case of switch/case here that check mainly ensures we ignore neverType as that's comparable to everything. neverType is specifically used within switchTypes to indicate that this switch statement has a default case (see getTypeOfSwitchClause here and hasDefaultClause here)
|
@typescript-bot test it |
|
@DanielRosenwasser Here are the results of running the user tests with tsc comparing Everything looks good! |
|
Hey @DanielRosenwasser, the results of running the DT tests are ready. Everything looks the same! |
|
@DanielRosenwasser Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@DanielRosenwasser Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
|
Hi @Andarist Is there anything blocking this pull request? |
fixes #61207