feat(es/transformer): Add edge_default_param bugfix hook#11504
feat(es/transformer): Add edge_default_param bugfix hook#11504
Conversation
This adds the `edge_default_param` bugfix to `swc_ecma_transformer` using the hook-based visitor pattern. This transform converts destructured parameters with default values to non-shorthand syntax, fixing an arguments-related bug in Edge 16 & 17. Changes: - Add `edge_default_param.rs` hook in `bugfix/` module - Add `enter_object_pat`/`exit_object_pat` hooks to `OrderedChain` - Update `BugfixOptions` with `edge_default_param` option Closes #10470 Co-authored-by: Donny/강동윤 <[email protected]>
|
|
|
Binary Sizes
Commit: f49f7c6 |
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
|
PR Review: feat(es/transformer): Add edge_default_param bugfix hookSummaryThis PR adds the Code Quality and Best Practices ✅Strengths:
Minor suggestion:
Potential Bugs or Issues
|
There was a problem hiding this comment.
Pull request overview
Adds a new Edge-specific bugfix hook to swc_ecma_transformer to rewrite destructured default parameters into non-shorthand object pattern syntax, addressing Edge 16/17 arguments-related behavior.
Changes:
- Introduces
BugfixOptions::edge_default_paramflag and wires bugfix hooks viaHookBuilder. - Adds
edge_default_paramhook implementation that rewrites{ a = 1 }to{ a: a = 1 }in object patterns. - Extends hook chaining support to forward
enter_object_pat/exit_object_patevents.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/swc_ecma_transformer/src/hook_utils.rs | Adds chained hook forwarding for ObjectPat to enable bugfix visitor callbacks. |
| crates/swc_ecma_transformer/src/bugfix/mod.rs | Adds edge_default_param option and composes bugfix hooks with HookBuilder. |
| crates/swc_ecma_transformer/src/bugfix/edge_default_param.rs | Implements the Edge default-param object pattern rewrite as a hook-based pass. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn hook(options: BugfixOptions) -> impl VisitMutHook<TraverseCtx> { | ||
| BugfixPass { options } | ||
| } | ||
| let hook = HookBuilder::new(NoopHook); | ||
|
|
||
| struct BugfixPass { | ||
| options: BugfixOptions, | ||
| } | ||
| // Edge default param: { a = 1 } -> { a: a = 1 } | ||
| let hook = hook.chain(if options.edge_default_param { | ||
| Some(self::edge_default_param::hook()) |
There was a problem hiding this comment.
The PR description says this change “Closes #10470”, but the linked issue is about reducing the number of AST visitors in the ecma minifier, while this PR adds a new bugfix hook in swc_ecma_transformer. If #10470 isn’t actually resolved by this change, please remove the auto-close reference or link the correct issue.
| #[derive(Debug, Default)] | ||
| #[non_exhaustive] | ||
| pub struct BugfixOptions {} | ||
| pub struct BugfixOptions { | ||
| /// Enable edge default param bugfix. | ||
| /// | ||
| /// Converts destructured parameters with default values to non-shorthand | ||
| /// syntax. This fixes the only arguments-related bug in ES | ||
| /// Modules-supporting browsers (Edge 16 & 17). | ||
| pub edge_default_param: bool, | ||
| } | ||
|
|
||
| impl BugfixOptions { | ||
| /// Returns true if any transform is enabled. | ||
| /// Currently no bugfixes are available. | ||
| pub fn is_enabled(&self) -> bool { | ||
| false | ||
| self.edge_default_param | ||
| } | ||
| } | ||
|
|
||
| pub fn hook(options: BugfixOptions) -> impl VisitMutHook<TraverseCtx> { | ||
| BugfixPass { options } | ||
| } | ||
| let hook = HookBuilder::new(NoopHook); | ||
|
|
||
| struct BugfixPass { | ||
| options: BugfixOptions, | ||
| } | ||
| // Edge default param: { a = 1 } -> { a: a = 1 } | ||
| let hook = hook.chain(if options.edge_default_param { | ||
| Some(self::edge_default_param::hook()) | ||
| } else { | ||
| None | ||
| }); | ||
|
|
||
| impl VisitMutHook<TraverseCtx> for BugfixPass {} | ||
| hook.build() | ||
| } |
There was a problem hiding this comment.
This introduces new behavior behind BugfixOptions::edge_default_param, but there are no tests exercising the hook-based implementation in swc_ecma_transformer (the existing tests in swc_ecma_compat_bugfixes cover the separate VisitMut pass, not this hook). Consider adding a small set of regression tests (e.g., snapshot tests) that run swc_ecma_transformer::Options with env.bugfix.edge_default_param = true/false and cover at least the default-value case, the no-default case, and a nested pattern case.
| EdgeDefaultParamPass { in_arrow: false } | ||
| } | ||
|
|
||
| struct EdgeDefaultParamPass { | ||
| in_arrow: bool, | ||
| } | ||
|
|
||
| impl VisitMutHook<TraverseCtx> for EdgeDefaultParamPass { | ||
| fn enter_arrow_expr(&mut self, _node: &mut ArrowExpr, _ctx: &mut TraverseCtx) { | ||
| self.in_arrow = true; | ||
| } | ||
|
|
||
| fn exit_arrow_expr(&mut self, _node: &mut ArrowExpr, _ctx: &mut TraverseCtx) { | ||
| self.in_arrow = false; | ||
| } | ||
|
|
||
| fn exit_object_pat(&mut self, n: &mut ObjectPat, _ctx: &mut TraverseCtx) { | ||
| if !self.in_arrow { | ||
| return; | ||
| } | ||
|
|
||
| for prop in n.props.iter_mut() { | ||
| if let ObjectPatProp::Assign(AssignPatProp { | ||
| value: Some(value), | ||
| key, | ||
| span, | ||
| .. | ||
| }) = prop | ||
| { | ||
| *prop = ObjectPatProp::KeyValue(KeyValuePatProp { | ||
| key: PropName::Ident(key.clone().into()), | ||
| value: AssignPat { | ||
| span: *span, | ||
| left: key.clone().into(), | ||
| right: value.clone(), | ||
| } | ||
| .into(), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
EdgeDefaultParamPass currently uses a single in_arrow boolean that is set for the entire ArrowExpr traversal. This makes the rewrite run for all ObjectPats anywhere inside an arrow (including in the body), even though the bugfix is documented as targeting destructured parameters only. It also becomes incorrect for nested arrows because exiting an inner arrow sets in_arrow = false while still inside the outer arrow.
To match the intended behavior (and the existing implementation in crates/swc_ecma_compat_bugfixes/src/edge_default_param.rs), restrict the rewrite to ArrowExpr.params only (e.g., perform the transformation directly in enter_arrow_expr by walking node.params), or alternatively track nesting with a counter/stack and only enable the rewrite while visiting the parameter list.
| EdgeDefaultParamPass { in_arrow: false } | |
| } | |
| struct EdgeDefaultParamPass { | |
| in_arrow: bool, | |
| } | |
| impl VisitMutHook<TraverseCtx> for EdgeDefaultParamPass { | |
| fn enter_arrow_expr(&mut self, _node: &mut ArrowExpr, _ctx: &mut TraverseCtx) { | |
| self.in_arrow = true; | |
| } | |
| fn exit_arrow_expr(&mut self, _node: &mut ArrowExpr, _ctx: &mut TraverseCtx) { | |
| self.in_arrow = false; | |
| } | |
| fn exit_object_pat(&mut self, n: &mut ObjectPat, _ctx: &mut TraverseCtx) { | |
| if !self.in_arrow { | |
| return; | |
| } | |
| for prop in n.props.iter_mut() { | |
| if let ObjectPatProp::Assign(AssignPatProp { | |
| value: Some(value), | |
| key, | |
| span, | |
| .. | |
| }) = prop | |
| { | |
| *prop = ObjectPatProp::KeyValue(KeyValuePatProp { | |
| key: PropName::Ident(key.clone().into()), | |
| value: AssignPat { | |
| span: *span, | |
| left: key.clone().into(), | |
| right: value.clone(), | |
| } | |
| .into(), | |
| }); | |
| } | |
| } | |
| EdgeDefaultParamPass {} | |
| } | |
| struct EdgeDefaultParamPass {} | |
| impl VisitMutHook<TraverseCtx> for EdgeDefaultParamPass { | |
| fn enter_arrow_expr(&mut self, node: &mut ArrowExpr, _ctx: &mut TraverseCtx) { | |
| fn transform_object_pat(n: &mut ObjectPat) { | |
| for prop in n.props.iter_mut() { | |
| if let ObjectPatProp::Assign(AssignPatProp { | |
| value: Some(value), | |
| key, | |
| span, | |
| .. | |
| }) = prop | |
| { | |
| *prop = ObjectPatProp::KeyValue(KeyValuePatProp { | |
| key: PropName::Ident(key.clone().into()), | |
| value: AssignPat { | |
| span: *span, | |
| left: key.clone().into(), | |
| right: value.clone(), | |
| } | |
| .into(), | |
| }); | |
| } | |
| } | |
| } | |
| fn transform_pat(pat: &mut Pat) { | |
| match pat { | |
| Pat::Object(obj) => { | |
| transform_object_pat(obj); | |
| } | |
| Pat::Assign(AssignPat { left, .. }) => { | |
| transform_pat(left); | |
| } | |
| Pat::Array(array) => { | |
| for elem in &mut array.elems { | |
| if let Some(inner) = elem { | |
| transform_pat(inner); | |
| } | |
| } | |
| } | |
| _ => {} | |
| } | |
| } | |
| for param in &mut node.params { | |
| transform_pat(param); | |
| } |
![kodiakhq[bot]](https://avatars.githubusercontent.com/in/29196?s=60&v=4){kind=small}
2 participants
Summary
edge_default_parambugfix toswc_ecma_transformerusing hook-based visitor patternTest plan
cargo check -p swc_ecma_transformerpassescargo test -p swc_ecma_compat_bugfixespasses (30 tests)Generated with Claude Code