-
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
visitNodesWithoutCopyingPositions
always makes a new NodeArray
#59137
Conversation
@typescript-bot perf test this |
@@ -8980,7 +8980,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { | |||
if (result) { | |||
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
createNodeArray
always 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 newNodeArray
is 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
createT
vsupdateT
,createNodeArray
doesn'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___NodeArray
functions that makes this distinction)