fix(es/minifier): respect ecma for iife temp vars#11873
Conversation
🦋 Changeset detectedLatest commit: 9830d18 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 not alter performance
Comparing 
Footnotes
|
![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: 364bf3f4cd
ℹ️ 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 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 address that feedback".
| kind: if self.options.ecma >= EsVersion::Es2015 { | ||
| VarDeclKind::Let | ||
| } else { | ||
| VarDeclKind::Var | ||
| }, |
There was a problem hiding this comment.
Switching synthesized temp declarations from let to var in invoke_iife breaks closure semantics for arrow-IIFE inlining inside loops when compress.ecma < 2015. In patterns like for (const k in obj) { out[k] = ((k)=>()=>k)(k) }, the extracted temp now becomes a function-scoped var, so all generated closures share one binding and observe the last iteration’s value instead of each iteration’s value. This is a runtime behavior regression (not just formatting) and is visible in the updated 8246 fixture shape where the captured temp is now var inside the loop body.
Useful? React with 👍 / 👎.
| kind: if self.options.ecma >= EsVersion::Es2015 { | ||
| VarDeclKind::Let | ||
| } else { | ||
| VarDeclKind::Var | ||
| }, |
There was a problem hiding this comment.
Using var here also removes block scoping for synthesized parameter temporaries, so an inlined arrow-IIFE inside a nested block can unexpectedly alias an existing outer binding with the same name. For example, var value = 1; { ((value)=>value)(2); } now lowers through inline_fn_param into assignments against a function-scoped var value, mutating the outer value (returning 2) even though the original arrow parameter should have remained block-local shadowing.
Useful? React with 👍 / 👎.
| kind: if self.options.ecma >= EsVersion::Es2015 { | ||
| VarDeclKind::Let | ||
| } else { | ||
| VarDeclKind::Var | ||
| }, |
There was a problem hiding this comment.
Lowering these synthesized temporaries to function-scoped var can make previously valid code fail to parse when the enclosing function already has a lexical binding with the same name. A valid case like function f(){ let x=0; { ((x)=>x)(1) } } can be rewritten with an injected var x, which conflicts with the existing let x in the same function scope and triggers Identifier 'x' has already been declared.
Useful? React with 👍 / 👎.
|
I believe the current behavior is fine as there're very few browsers that supports arrow but not let/const. |
its actually about performance let and const is much slower in some js engine(like quickjs and safari let | const is about 10x+ slower than var during parsing in primjs engine(which is a modified version of quickjs), when I replace let | const to var in our code, the whole parse time reduced from 453ms to 264ms for our code |
3 participants
Description:
When compressing arrow IIFEs, the minifier can synthesize temporary variable declarations for extracted parameters. That path always emitted
let, even whencompress.ecmawas ES5.This updates the synthesized declaration kind to use
varbelow ES2015 and keepletfor ES2015+, then adds an ES5 fixture covering the arrow IIFE temp-var path. Related snapshots were updated for the same behavior change.BREAKING CHANGE:
None.
Related issue (if exists):
N/A
Testing:
git submodule update --init --recursivecargo fmt --allcargo test -p swc_ecma_minifiercargo clippy --all --all-targets -- -D warnings