Derive tuple labels for rest elements from array binding patterns#59045
Merged
Derive tuple labels for rest elements from array binding patterns#59045
Conversation
Member
|
@typescript-bot test it |
Collaborator
Collaborator
|
Hey @DanielRosenwasser, the results of running the DT tests are ready. Everything looks the same! |
Collaborator
|
@DanielRosenwasser Here are the results of running the user tests with tsc comparing Everything looks good! |
Collaborator
|
@DanielRosenwasser Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Collaborator
|
@DanielRosenwasser Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
Contributor
|
I already have a very related change to this open here. Both PRs touch the same area of the code and solve very overlapping problems, I think it would make sense to merge both |
Member
|
Seems fine to me, but does need a sync from main and baseline update. |
a497918 to
ae645c3
Compare
jakebailey
approved these changes
Jul 17, 2024
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
While working on #58243, a community member asked if we could label the tuple we use to properly type arguments to the
next()methods of iterators and generators. A brief experiment showed that directly labeling the tuple element resulted in the creation of thousands of new types in some tests since iterator/generator instantiations would produce new named tuple elements since named tuple elements are cached far less frequently.An alternative to that approach would be to use an additional source for the label when the label comes from a rest parameter whose name is actually an
ArrayBindingPattern:Since this is merely treated as an additional labeling source, there is no need to synthesize additional types for named tuple elements.
This also improves label inference in other cases such as elision and object assignment pattern elements:
Related #58243 (comment)
Fixes #56289
Closes #57619