fix(transformer/async-to-generator): move only parameter bindings#22503
Merged
Conversation
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes async-to-generator semantic scope handling so only actual parameter bindings are moved when method parameters remain on the wrapper function.
Changes:
- Adds binding-focused visitor overrides for formal parameters, rest parameters, and assignment patterns.
- Updates semantic coverage snapshots to remove resolved binding/symbol-scope mismatches.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/oxc_transformer/src/es2017/async_to_generator.rs |
Narrows BindingMover traversal to avoid visiting default initializer expressions. |
tasks/coverage/snapshots/semantic_test262.snap |
Updates expected semantic mismatch output for affected Test262 async generator method cases. |
Comments suppressed due to low confidence (1)
crates/oxc_transformer/src/es2017/async_to_generator.rs:936
- The left side of an assignment pattern can itself be an object binding pattern with computed property keys. Calling the generic
visit_binding_patternhere still descends into those key expressions and can move bindings declared inside nested function/class expressions, so this should use binding-position-only traversal for the pattern structure.
self.visit_binding_pattern(&pattern.left);
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
crates/oxc_transformer/src/es2017/async_to_generator.rs:942
- Skipping computed key expressions here prevents their bindings from being moved, but any scopes created inside those keys are still parented to the old generator scope even though the parameter remains on the wrapper method. The new
class/computed-key-bindingfixture is still recorded as a semantic failure inoxc.snap.mdwith a scope-parent mismatch, so the transformed scope tree remains inconsistent; the first-level scopes inside skipped key/initializer expressions need to be reparented to the wrapper scope while leaving their bindings in place.
fn visit_binding_property(&mut self, property: &BindingProperty<'a>) {
// Computed keys are expressions and can contain their own scopes. Only the property value
// is a binding position.
self.visit_binding_pattern(&property.value);
crates/oxc_transformer/src/es2017/async_to_generator.rs:936
- The RHS default expression can also introduce function/class scopes, and those scopes still need to follow the parameter pattern when it remains on the wrapper method. Only skipping the RHS for binding moves leaves nested default scopes parented to the old generator scope (the updated Test262 snapshot still shows scope-parent mismatches for
*-ptrn-*-init-fn-name-*cases), so this path also needs scope reparenting while keeping the RHS bindings untouched.
fn visit_assignment_pattern(&mut self, pattern: &AssignmentPattern<'a>) {
// Default values are expressions, not parameter bindings. Visiting only the left-hand side
// avoids moving bindings declared inside the initializer expression.
self.visit_binding_pattern(&pattern.left);
Contributor
Author
Merge activity
|
graphite-app Bot
pushed a commit
that referenced
this pull request
May 17, 2026
…2503) Previously, given: ```js class C { async method(x = function y() {}) {} } ``` The async-to-generator transform generates: ```js class C { method(x = function y() {}) { return babelHelpers.asyncToGenerator(function* () {})(); } } ``` The method parameter stays on the outer wrapper method, while only the original body moves into the inner generator function. Before this change, `BindingMover` walked the full parameter AST while moving those parameter bindings. That meant it also visited default initializer expressions, so bindings created inside the initializer, like `y` above, could be moved out of their own function/class scope and into the wrapper method scope. After transform, rebuilt semantics would disagree with the transformed scope tree because the parameter binding was correct, but nested initializer bindings had the wrong owning scope. This PR makes `BindingMover` only visit the binding positions for formal parameters, rest parameters, and the left side of assignment patterns. It skips initializer expressions so nested function/class bindings stay in the scopes created for those expressions.
56fff4d to
af880c9
Compare
…2503) Previously, given: ```js class C { async method(x = function y() {}) {} } ``` The async-to-generator transform generates: ```js class C { method(x = function y() {}) { return babelHelpers.asyncToGenerator(function* () {})(); } } ``` The method parameter stays on the outer wrapper method, while only the original body moves into the inner generator function. Before this change, `BindingMover` walked the full parameter AST while moving those parameter bindings. That meant it also visited default initializer expressions, so bindings created inside the initializer, like `y` above, could be moved out of their own function/class scope and into the wrapper method scope. After transform, rebuilt semantics would disagree with the transformed scope tree because the parameter binding was correct, but nested initializer bindings had the wrong owning scope. This PR makes `BindingMover` only visit the binding positions for formal parameters, rest parameters, and the left side of assignment patterns. It skips initializer expressions so nested function/class bindings stay in the scopes created for those expressions.
af880c9 to
ecfd3ca
Compare
camc314
pushed a commit
that referenced
this pull request
May 18, 2026
### 🐛 Bug Fixes - 0f26de6 ecmascript: Resolve identifier value type via tracked constants (#22234) (Alexander Lichter) - c27a8cf minifier: Normalize `{ x: x }` shorthand so adjacent-if merge is idempotent (#22401) (Dunqing) - e431a0e parser: Break extends clause loop on fatal error (#22517) (Boshen) - e9ec7c6 minifier: Fold optional chains by base nullishness (#22236) (Alexander Lichter) - e6090e7 transformer: Keep enum IIFE when a non-inlinable value reference remains (#22501) (Dunqing) - 931b7d6 transformer: Inline const enum members through type-cast wrappers (#22500) (Dunqing) - b9615b2 codegen: Preserve string quotes in require() calls during minification (#22475) (zennnnnnn11) - c73c159 transformer/async-to-generator: Reparent parameter initializer scopes (#22507) (camc314) - ecfd3ca transformer/async-to-generator: Move only parameter bindings (#22503) (camc314) - 3ce3431 transformer/explicit-resource-managment: Preserve shadowed for-head block (#22451) (camc314) ### ⚡ Performance - ce92c6c semantic: `#[inline]` `Scoping::get_binding` (#22414) (Dunqing) - 98be95c regular_expression: Track regex flags via bitflags (#22427) (Boshen) - dbbc059 jsdoc: Skip should_attach_jsdoc when no remaining comments (#22409) (Boshen) - 217d7d8 minifier: Index `SymbolValues` by `SymbolId` (#22441) (Dunqing) - d782b78 minifier: Use BitSet for LiveUsageCollector live references (#22425) (Boshen)
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously, given:
The async-to-generator transform generates:
The method parameter stays on the outer wrapper method, while only the original body moves into the inner generator function.
Before this change,
BindingMoverwalked the full parameter AST while moving those parameter bindings. That meant it also visited default initializer expressions, so bindings created inside the initializer, likeyabove, could be moved out of their own function/class scope and into the wrapper method scope.After transform, rebuilt semantics would disagree with the transformed scope tree because the parameter binding was correct, but nested initializer bindings had the wrong owning scope.
This PR makes
BindingMoveronly visit the binding positions for formal parameters, rest parameters, and the left side of assignment patterns. It skips initializer expressions so nested function/class bindings stay in the scopes created for those expressions.