-
Notifications
You must be signed in to change notification settings - Fork 13.2k
visitNodesWithoutCopyingPositions always makes a new NodeArray
#59137
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
Conversation
|
@typescript-bot perf test this |
src/compiler/checker.ts
Outdated
| if (result.pos !== -1 || result.end !== -1) { | ||
| if (result === nodes) { | ||
| result = factory.createNodeArray(nodes, nodes.hasTrailingComma); | ||
| result = factory.createNodeArray([...nodes], nodes.hasTrailingComma); |
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 believe slice() tends to be a little faster than spreading since engines have to do shenanigans to support non-array iterables.
| result = factory.createNodeArray([...nodes], nodes.hasTrailingComma); | |
| result = factory.createNodeArray(nodes.slice(), nodes.hasTrailingComma); |
|
@iisaduan Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@typescript-bot perf test this |
|
@iisaduan Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@typescript-bot cherry pick this to release-5.5 |
|
@typescript-bot cherry-pick this to release-5.5 |
|
Hey, @DanielRosenwasser! I've created #59179 for you. |
…e-5.5 (#59179) Co-authored-by: Isabel Duan <[email protected]>
fixes #59115
The original call assumed
createNodeArrayalways makes a newNodeArray, however, in some instances (when there are no changes to theNodeArray), the original array returns. Unpacking the array makes sure that a newNodeArrayis always created.(Side note: As a lot of nodeFactory functions make a distinction between when it is always new node vs it is possible to return the old one, ie
createTvsupdateT,createNodeArraydoesn't currently follow that convention. A bigger version of this PR would be to change the behavior ofcreateNodeArray(such as #59135, but it could/would have other side effects), or to make two___NodeArrayfunctions that makes this distinction)