fix(es/compat): Preserve array rest when lowering nested object rest - #12352
fix(es/compat): Preserve array rest when lowering nested object rest#12352magic-akari wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 9bcf515 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will improve performance by 9.62%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | script[1024] |
244.4 ns | 215.3 ns | +13.55% |
| ⚡ | script[16] |
244.4 ns | 215.3 ns | +13.55% |
| ⚡ | es/large/minify/libraries/d3 |
484.5 ms | 474.2 ms | +2.15% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/issue-12339 (9bcf515) with main (ff6a3c3)
Footnotes
-
61 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bcf51551f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| Some(Pat::Rest(rest)) => { | ||
| let nested = rest.arg.take(); | ||
| *rest.arg = split_ref.clone().into(); | ||
| *nested |
There was a problem hiding this comment.
When the nested rest pattern assigns to the identifier used as the RHS, this new branch returns the overwritten binding rather than the original assignment value. For example, const original = source; const result = ([...{ 0: source, ...tail }] = source); should leave result === original, but the transform emits the equivalent of ([..._ref] = source, { 0: source } = _ref, ..., source), so the final expression evaluates to the newly assigned element. Capture the RHS in a temporary for this assignment path so the sequence returns its original value.
Useful? React with 👍 / 👎.
| Some(Pat::Rest(rest)) => { | ||
| let nested = rest.arg.take(); | ||
| *rest.arg = split_ref.clone().into(); | ||
| *nested |
There was a problem hiding this comment.
For a lexical loop head such as let x = [[1]]; for (let [...{ ...x }] of x) {}, the iterable expression must resolve to the new loop binding and throw due to its TDZ. This newly handled pattern is subsequently processed by transform_for_loop, which replaces the head binding with a temporary and declares x inside the body, leaving the iterable expression outside x's lexical scope; the generated loop can therefore read the outer/global x and execute instead. Keep the original head-binding environment around evaluation of the iterable expression or otherwise preserve the TDZ.
Useful? React with 👍 / 👎.
| Some(Pat::Rest(rest)) => { | ||
| let nested = rest.arg.take(); | ||
| *rest.arg = split_ref.clone().into(); | ||
| *nested |
There was a problem hiding this comment.
In sloppy functions that have an earlier simple parameter, lowering this newly supported pattern to a plain temporary makes the entire parameter list simple and creates a mapped arguments object. For example, function f(a, [...{ ...x }]) { a = 2; return arguments[0]; } must return the original first argument, but after ParamCollector replaces the pattern with _1, assigning to a also updates arguments[0], so the transformed function returns 2. Preserve a non-simple parameter form while extracting the nested pattern so the function's arguments remain unmapped.
Useful? React with 👍 / 👎.
1 participant
Description:
Fix a panic when lowering patterns such as
const [...{ ...y }] = [1, 2]to ES5.Preserve the array rest element when splitting the pattern, replacing only its argument with a temporary binding. This ensures remaining elements are collected before lowering the nested object rest and prevents emitting an invalid standalone rest declaration.
Add fixture and execution coverage for direct and nested patterns, assignments, function parameters, for-of bindings, and iterator consumption.
Related issue (if exists):