Propagate the error any type in union and intersection construction #58610
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
typescript-bot
added
Author: Team
For Uncommitted Bug
|
Is this at all related to #56429? |
|
It's related insofar as better |
sandersn
approved these changes
May 22, 2024
| @@ -17303,7 +17304,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| if (unionReduction !== UnionReduction.None) { | |||
| if (includes & TypeFlags.AnyOrUnknown) { | |||
| return includes & TypeFlags.Any ? | |||
| includes & TypeFlags.IncludesWildcard ? wildcardType : anyType : | |||
| includes & TypeFlags.IncludesWildcard ? wildcardType : | |||
| includes & TypeFlags.IncludesError ? errorType : anyType : | |||
| unknownType; | |||
There was a problem hiding this comment.
These ternaries are officially off the chain now.
There was a problem hiding this comment.
🤷♀️ I just write the ternaries, I let the formatter figure out how it wants to indent them.
| @@ -17637,7 +17640,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
| return neverType; | |||
| } | |||
| if (includes & TypeFlags.Any) { | |||
| return includes & TypeFlags.IncludesWildcard ? wildcardType : anyType; | |||
| return includes & TypeFlags.IncludesWildcard ? wildcardType : includes & TypeFlags.IncludesError ? errorType : anyType; | |||
There was a problem hiding this comment.
it would be nice to have multiple lines here
| @@ -6185,6 +6185,8 @@ export const enum TypeFlags { | |||
| StringMapping = 1 << 28, // Uppercase/Lowercase type | |||
| /** @internal */ | |||
| Reserved1 = 1 << 29, // Used by union/intersection type construction | |||
| /** @internal */ | |||
| Reserved2 = 1 << 30, // Used by union/intersection type construction | |||
There was a problem hiding this comment.
remind me, do we have 31 bits available?
|
@typescript-bot test it |
|
@jakebailey Here are the results of running the user tests comparing Something interesting changed - please have a look. Details
|
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@typescript-bot pack this |
|
Hey @weswigham, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@jakebailey Here are the results of running the top 400 repos comparing Something interesting changed - please have a look. Details
|
|
@typescript-bot user test this |
|
@weswigham Here are the results of running the user tests comparing Something interesting changed - please have a look. Details
|
|
Something is funky with @jakebailey is the runner silently failing to install/link dependencies correctly for that user test or something? That's the only thing I could think of, which is why I reran the test. And it would make sense - these kinda of errors are what I'd expect to see if some intersections in class base types contained unresolved types that now get propagated out (instead of quietly |
|
The test thing infers packages and tsconfigs and attempts to install them and run on each of them, so it's possible that it's just doing things "wrong" for that repo. We can convert it to a script test or something where it more clearly tries to run things, but we'll lose the inference of configs and such. I'm not sure I have the time to try and fix the root problem, though. |
|
The project does have one diff in the real output - a new error - but it's a follow-on caused by what I said above - a class has an error type intersected into its' base types, so all its' base types get erased now, rather than Big downside of using |
|
Useful investigation regardless, even if there wasn't really much related to this issue, disabling |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
You might not think this is the obvious fix for the issue, but it is! Type parameter constraints, when they see a non-error
any, remap it tounknown(there are historical reasons for this). So by swallowing the error-any(and replacing it with a normalany), union and intersection construction resulted in the constraint calculation logic not witnessing the error type, and thus remapping the constraint (and, thus, making the constraint from the type node not match the cached constraint on the type parameter itself!)This should also just behave a bit better with respect to follow on errors in some other cases, too.
Fixes #58598