-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Add explicit type variance specifiers to Promise, Map, Set #62973
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
base: main
Are you sure you want to change the base?
Conversation
|
The TypeScript team hasn't accepted the linked issue #62954. If you can get it accepted, this PR will have a better chance of being reviewed. |
|
@microsoft-github-policy-service agree |
|
I'm 99% certain we can't do this, but, curious @typescript-bot test it |
|
@akaltar sorry, but this is wrong in so far as it subverts type-safety. take Map for example: more generally: as long as you have a mutable collection, you cannot define its values covariant. |
|
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
|
@jakebailey Here are the results of running the user tests with tsc comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Something interesting changed - please have a look. Details
|
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@jakebailey Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
Fixes #62954
In theory this could improve the performance of type checking in certain scenarios by avoiding a call to
getVariancesWorkeras measured in the perfetto profiles.I've opted to only include some of the most commonly used types, so the added parsing time doesn't outweigh the benefits
I wasn't sure if the explicit specifier needs to be on every expansion of an interface or only the first one, so I've added it to all of them from es5 to esnext, but depending on how the compiler handles it, it could be applied only to the first occurrence of the interface