In createNodeArray, create a fresh array (2)#28441
Conversation
ghost
requested a review
from 
| return elements; | ||
| } | ||
| // elements is a ReadonlyArray, so create a fresh array before adding properties. | ||
| elements = elements.slice(); |
There was a problem hiding this comment.
Is there a scenario that breaks because of this? I think from below code looks like the intention of the code is to make elements array into node array?
There was a problem hiding this comment.
That is generally the purpose of this function, yes. elements was not always a ReadonlyArray, but now that it is it makes sense to make a fresh copy. We should verify the performance/memory characteristics of this change, however.
There was a problem hiding this comment.
Although, mutating the array this way does not actually change its "ReadonlyArray"-ness, just promotes it to a Node Array (which is also a ReadonlyArray).
ghost
assigned
|
Unclear what this fixes and has conflicts now |
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
4 participants
Refreshing #20518