- Notifications
You must be signed in to change notification settings - Fork 13.2k
Empty intersection reduction#31838
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
Empty intersection reduction #31838
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ahejlsberg commented Jun 9, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
ahejlsberg commented Jun 9, 2019
@typescript-bot test this |
typescript-bot commented Jun 9, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 015f4c5. You can monitor the build here. It should now contribute to this PR's status checks. |
typescript-bot commented Jun 9, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 015f4c5. You can monitor the build here. It should now contribute to this PR's status checks. |
ahejlsberg commented Jun 9, 2019
RWC test run is clean. DT test run has one minor difference in |
jack-williams commented Jun 9, 2019
Does this still support branding via object literals? typeTag<T>=T&{readonlybrand: unique symbol};typeTaggedNumber=Tag<number>;// not never |
ahejlsberg commented Jun 9, 2019
@jack-williams It does, except you can't brand |
ahejlsberg commented Jun 9, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@jack-williams The only thing that changes with this PR is when we reduce away empty intersections. We now do it upon construction but we used to do it only when an intersection was put in a union type. The actual patterns we reduce away (or don't reduce away) remain the same. |
jack-williams commented Jun 9, 2019
Ah, that makes it clearer. Thank you! |
| if (type.flags & TypeFlags.Unit && includes & TypeFlags.Unit){ | ||
| // We have seen two distinct unit types which means we should reduce to an | ||
| // empty intersection. Adding TypeFlags.NonPrimitive causes that to happen. | ||
| includes |= TypeFlags.NonPrimitive; |
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.
Counterexample:
enumA{Zero=0,One=1}typeZeroish<T>=T&0;typeShouldNotBeNever=Zeroish<A.Zero>|never;we handle this incorrectly in master already - I'm just not a fan of propagating the logical error.
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.
So is the reason for that that the code highlighted considers A.Zero and 0 to be distinct? I'm just trying to ascertain where in that example above the never creeps in.
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.
@weswigham I think it is debatable whether we want to consider literals and literal enum members with the same underlying value to be intersectable. But either way, it is an issue that should be covered in a separate PR. Here we're just concerned with changing when we reduce, not how.
ahejlsberg commented Jun 11, 2019
@typescript-bot run dt |
typescript-bot commented Jun 11, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 015f4c5. You can monitor the build here. It should now contribute to this PR's status checks. |
ahejlsberg commented Jun 11, 2019
RWC and DT test runs are both clean now. |
weswigham left a comment
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.
I expect we'll get some reports about the using-an-enum-to-tag break upon release - we should remember to include this in the breaking change log.
dharnil commented May 31, 2024
Hi I wanted to check to see if this has already been resolved as I am still facing this issue. |
Utilizing [TypeScript intersection](microsoft/TypeScript#31838) to simplify config types. Config keys and payload can now always be `undefined`.
With this PR we reduce empty intersections to
neverimmediately upon construction. For example, instead of preserving the typestring & numberand only reducing it away in union types we now immediately reduce it tonever.Some examples of types that now immediately reduce to never:
In #18438 we chose to preserve empty intersection types (a) to make it easier to understand how they originated and (b) to partially preserve the ability to "tag" primtive types using enum types (e.g.
string & TagwhereTagis an enum). In practice this doesn't seem to outweigh the confusion these types bring about, in particular because there are subtle behavioral differences betweenneverand types likestring & numbereven though they really should be the same. The differences include:neveris assignable to anything whereasstring & numberis assignable only tostringornumber.anyis not assignable toneverbut is assignable tostring & number.Twith no index signatures,T[never]resolves toneverwhereasT[string & number]is an error.These differences have no practical value. Particularly confusing is that
string & booleanreduces tonever(because it normalizes tostring & true | string & false, which then reduces tonever), whereasstring & numbersticks around and behaves differently.Fixes#31663.