fix(es/minifier): preserve args for destructured callbacks#11830
fix(es/minifier): preserve args for destructured callbacks#11830kdy1 merged 4 commits intoswc-project:mainfrom
Conversation
![kodiakhq[bot]](https://avatars.githubusercontent.com/in/29196?s=60&v=4){kind=small}
|
There was a problem hiding this comment.
Pull request overview
Fixes a SWC minifier regression where call-argument dropping could incorrectly remove arguments when a callback is obtained via destructuring and later conditionally replaced with a zero-arity fallback.
Changes:
- Update usage analysis to treat variables introduced via destructuring patterns as having unknown callable arity (
param_count = Unknown). - Preserve precise arity inference for direct identifier initializers that are function/arrow expressions.
- Add a regression fixture for issue #11829 validating the runtime behavior (
cb("value")still receives its argument).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/swc_ecma_minifier/src/usage_analyzer/analyzer/mod.rs | Ensures destructured bindings are marked with unknown arity to prevent unsafe argument dropping. |
| crates/swc_ecma_minifier/tests/terser/compress/drop_unused/issue_11829/input.js | Repro case demonstrating destructured callback with conditional fallback. |
| crates/swc_ecma_minifier/tests/terser/compress/drop_unused/issue_11829/config.json | Compressor configuration reproducing the problematic optimization scenario. |
| crates/swc_ecma_minifier/tests/terser/compress/drop_unused/issue_11829/expected.stdout | Runtime assertion to catch missing-argument behavior (expects PASS). |
| crates/swc_ecma_minifier/tests/terser/compress/drop_unused/issue_11829/output.js | Empty output golden to make this fixture validate runtime output rather than exact printed code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Destructuring pulls values from an external object or iterable, so the | ||
| // callable arity of each binding is not known from this declaration. |
There was a problem hiding this comment.
The comment claims destructuring always pulls values from an external object/iterable, but destructuring can also read from literals (e.g. let { cb } = { cb(){} }). Consider rewording to reflect the real invariant: for destructured bindings, callable arity can't be inferred reliably from the declarator, so the binding’s param_count is forced to Unknown for correctness.
| // Destructuring pulls values from an external object or iterable, so the | |
| // callable arity of each binding is not known from this declaration. | |
| // For destructured bindings, the callable arity of each bound value cannot | |
| // be inferred reliably from the declarator, so conservatively mark it unknown. |
Binary Sizes
Commit: 76bf6e8 |
Merging this PR will improve performance by 2.06%
Performance Changes
Comparing 
Footnotes
|
|
Maybe we should tech AI to respect MSRV. Also never add tests to terser tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| // Destructuring pulls values from an external object or iterable, so the | ||
| // callable arity of each binding is not known from this declaration. | ||
| for id in find_pat_ids(&decl.name) { | ||
| self.data | ||
| .var_or_default(id) | ||
| .store_param_count(Value::Unknown); |
| } else { | ||
| // Destructuring pulls values from an external object or iterable, so the | ||
| // callable arity of each binding is not known from this declaration. | ||
| for id in find_pat_ids(&decl.name) { | ||
| self.data | ||
| .var_or_default(id) | ||
| .store_param_count(Value::Unknown); |
**Description:** Adds agent guidance requested from PR review feedback: - Respect the project MSRV when writing Rust code. - Keep new swc_ecma_minifier regression coverage out of tests/terser and prefer SWC-owned fixtures such as tests/fixture/issues. This is documentation-only and helps future automated edits choose the right compatibility and fixture behavior. **Related issue:** Refs #11830 (comment)
|
Thank you! |
Description:
Fixes a minifier regression where
unused/call argument optimization could drop arguments for a callback loaded from destructuring and then conditionally assigned a zero-arity fallback. Destructured bindings with initializers now keep callback arity unknown, while direct function/arrow initializers still retain precise arity inference.Added a regression fixture for the reported
cb("value")case.BREAKING CHANGE:
None.
Related issue (if exists):
Fixes #11829