{} & null and {} & undefined should always be never #50553
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at dd32cf8. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at dd32cf8. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at dd32cf8. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at dd32cf8. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at dd32cf8. You can monitor the build here. Update: The results are in! |
|
I'm willing to join |
|
@ahejlsberg Here are the results of running the user test suite comparing Everything looks good! |
|
@ahejlsberg Here they are:Comparison Report - main..50553
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
|
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
|
Tests are all clean and perf looks good. Ready to merge. |
| type NonNullableNew<T> = T & {}; | ||
| type NonNullableOld<T> = T extends null | undefined ? never : T; | ||
|
|
||
| type IsNullWithoutStrictNullChecks = NonNullableNew<null>; |
There was a problem hiding this comment.
nit: this name seems to be confusing to me since it doesn't actually describe the expected result (if I understand correctly) but rather it refers to what was a bug
| @@ -0,0 +1,18 @@ | |||
| // @strict: false | |||
There was a problem hiding this comment.
I think it would be good to actually test these behaviors in both strict on and off.
| // @strict: false | |
| // @strict: true,false |
Then rename the test to nonNullablesAndObjectIntersections.ts
|
@typescript-bot perf test faster |
|
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at dd32cf8. You can monitor the build here. Update: The results are in! |
|
@DanielRosenwasser Here they are:Comparison Report - main..50553
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @@ -15166,7 +15166,7 @@ namespace ts { | |||
| return includes & TypeFlags.IncludesWildcard ? wildcardType : anyType; | |||
| } | |||
| if (!strictNullChecks && includes & TypeFlags.Nullable) { | |||
There was a problem hiding this comment.
Can't we try to delete this whole !strictNullChecks branch entirely? Or are you still fine with undefined & null being weirdly undefined in non-strict but never in strict?
There was a problem hiding this comment.
We need that check for cases where the intersection is a singleton null or undefined. undefined & null is already resolved to never by an earlier check.
There was a problem hiding this comment.
Oh, yeah, duh, I know why we can just delete this branch - 15 lines up there's a strictNullChecks only branch that does this already for strict mode - we can just delete both that strictNullChecks gate and this entire conditional.
There was a problem hiding this comment.
Nah, that branch also resolves null & object and null & { foo: string } to never, which we don't want to do in non-strictNullChecks mode.
There was a problem hiding this comment.
But why not? If we're already adjusting null & {}, isn't that just a logical followup? null & {x} seems even more suspect than null & {}.
There was a problem hiding this comment.
The whole premise of null and undefined being in the domain of every type is suspect. But that's non-strictNullChecks. My only objective here is to better align with pre-4.8 behavior in a corner case.
There was a problem hiding this comment.
I would be open to seeing what happens and considering such a change for 4.9, but I think this workaround for 4.8 is okay for now.
|
@typescript-bot cherry-pick this to release-4.8 |
|
Heya @andrewbranch, I've started to run the task to cherry-pick this into |
|
Hey @andrewbranch, I've opened #50589 for you. |
|
@ahejlsberg ready to merge? |
Fixes #50519.