-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Retain substitution types through instantiation if possible #30059
Conversation
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.
Code looks good and the justification sounds theoretically good. Since I'm the middle of the current ember break from variance testing on conditional types, though, I can't help be surprised that nothing breaks. This PR would be a good candidate for the "bot, test DT on this" feature once you get it working.
@typescript-bot test this & @typescript-bot run dt |
Heya @weswigham, I've started to run the extended test suite on this PR at 4fc9428. You can monitor the build here. It should now contribute to this PR's status checks. |
Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at 4fc9428. You can monitor the build here. It should now contribute to this PR's status checks. |
@RyanCavanaugh the bot has learned how to run DT tests on PRs. It takes like an hour and a half; use it sparingly~ (Also is DT is dirty, as it is today, I think you'll need to comb through the log to see what's a new failure by hand, though maybe @sandersn knows more) |
DT looks good~ |
Fixes #29677
The alias lacked the correct constraint because the constraint was implied by the substitution type, which disappeared when the type alias was instantiated with the outer type parameters. The correct behavior is to retain substitution types through instantiation if their type variable is still a type variable upon instantiation.
This required correctly handling substitution types on the target side in inference (other tests needed the change to continue to work as expected), naturally, since, eg, an
Extract<K, string>
's true branch is now going to look like a substitution ofK
instead ofK
directly.