Skip to content

Commit

Permalink
fix #2809: an obscure identifier minification bug
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jun 25, 2023
1 parent e40284c commit a7236e4
Showing 4 changed files with 63 additions and 3 deletions.
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -35,6 +35,34 @@
}
```

* Fix an obscure identifier minification bug ([#2809](https://github.com/evanw/esbuild/issues/2809))

Function declarations in nested scopes behave differently depending on whether or not `"use strict"` is present. To avoid generating code that behaves differently depending on whether strict mode is enabled or not, esbuild transforms nested function declarations into variable declarations. However, there was a bug where the generated variable name was not being recorded as declared internally, which meant that it wasn't being renamed correctly by the minifier and could cause a name collision. This bug has been fixed:

```js
// Original code
const n = ''
for (let i of [0,1]) {
function f () {}
}

// Old output (with --minify-identifiers --format=esm)
const f = "";
for (let o of [0, 1]) {
let n = function() {
};
var f = n;
}

// New output (with --minify-identifiers --format=esm)
const f = "";
for (let o of [0, 1]) {
let n = function() {
};
var t = n;
}
```
## 0.18.8
* Implement transforming `async` generator functions ([#2780](https://github.com/evanw/esbuild/issues/2780))
6 changes: 3 additions & 3 deletions internal/bundler_tests/snapshots/snapshots_default.txt
Original file line number Diff line number Diff line change
@@ -4750,21 +4750,21 @@ console.log((0, import_demo_pkg.default)());
TestNonDeterminismIssue2537
---------- /out.js ----------
// entry.ts
function b(o, e) {
function i(o, e) {
let r = "teun";
if (o) {
let u = function(n) {
return n * 2;
}, t = function(n) {
return n / 2;
};
var i = u, a = t;
var b = u, f = t;
r = u(e) + t(e);
}
return r;
}
export {
b as aap
i as aap
};

================================================================================
1 change: 1 addition & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
@@ -8469,6 +8469,7 @@ func (p *parser) visitStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt {

// Also write the function to the hoisted sibling symbol if applicable
if hoistedRef, ok := p.hoistedRefForSloppyModeBlockFn[s.Fn.Name.Ref]; ok {
p.recordDeclaredSymbol(hoistedRef)
p.recordUsage(s.Fn.Name.Ref)
varDecls = append(varDecls, js_ast.Decl{
Binding: js_ast.Binding{Loc: s.Fn.Name.Loc, Data: &js_ast.BIdentifier{Ref: hoistedRef}},
31 changes: 31 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
@@ -3818,6 +3818,37 @@ tests.push(
}),
)

// Check for an obscure bug with minification, symbol renaming, and sloppy
// nested function declarations: https://github.com/evanw/esbuild/issues/2809.
// Previously esbuild generated the following code:
//
// let f = 0;
// for (let l of [1, 2]) {
// let t = function(o) {
// return o;
// };
// var f = t;
// f += t(l);
// }
// if (f !== 3)
// throw "fail";
//
// Notice how "f" is declared twice, leading to a syntax error.
for (const flags of [[], ['--minify']]) {
tests.push(
test(['in.js', '--outfile=node.js', '--format=esm'].concat(flags), {
'in.js': `
let total = 0
for (let value of [1, 2]) {
function f(x) { return x }
total += f(value)
}
if (total !== 3) throw 'fail'
`,
}),
)
}

// Test hoisting variables inside for loop initializers outside of lazy ESM
// wrappers. Previously this didn't work due to a bug that considered for
// loop initializers to already be in the top-level scope. For more info

0 comments on commit a7236e4

Please sign in to comment.