Skip to content

Commit

Permalink
fix minify bug with variable inlining and this
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jun 20, 2022
1 parent 1dd274d commit f3c6248
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 0 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,24 @@

Notice how the property `foo` is always used unquoted but the property `bar` is always used quoted, so `foo` should be consistently mangled while `bar` should be consistently not mangled.

* Fix a minification bug regarding `this` and property initializers

When minification is enabled, esbuild attempts to inline the initializers of variables that have only been used once into the start of the following expression to reduce code size. However, there was a bug where this transformation could change the value of `this` when the initializer is a property access and the start of the following expression is a call expression. This release fixes the bug:

```js
// Original code
function foo(obj) {
let fn = obj.prop;
fn();
}

// Old output (with --minify)
function foo(f){f.prop()}

// New output (with --minify)
function foo(o){let f=o.prop;f()}
```

## 0.14.46

* Add the ability to override support for individual syntax features ([#2060](https://github.com/evanw/esbuild/issues/2060), [#2290](https://github.com/evanw/esbuild/issues/2290), [#2308](https://github.com/evanw/esbuild/issues/2308))
Expand Down
9 changes: 9 additions & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -8285,6 +8285,15 @@ func (p *parser) substituteSingleUseSymbolInExpr(
}

case *js_ast.ECall:
// Don't substitute something into a call target that could change "this"
_, isDot := replacement.Data.(*js_ast.EDot)
_, isIndex := replacement.Data.(*js_ast.EIndex)
if isDot || isIndex {
if id, ok := e.Target.Data.(*js_ast.EIdentifier); ok && id.Ref == ref {
break
}
}

if value, status := p.substituteSingleUseSymbolInExpr(e.Target, ref, replacement, replacementCanBeRemoved); status != substituteContinue {
e.Target = value
return expr, status
Expand Down
14 changes: 14 additions & 0 deletions internal/js_parser/js_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4315,6 +4315,20 @@ func TestMangleInlineLocals(t *testing.T) {
expectPrintedMangleTarget(t, 2015, "(x => { let y = x; throw y ?? z })()", "((x) => {\n let y = x;\n throw y != null ? y : z;\n})();\n")
expectPrintedMangleTarget(t, 2015, "(x => { let y = x; y.z ??= z })()", "((x) => {\n var _a;\n let y = x;\n (_a = y.z) != null || (y.z = z);\n})();\n")
expectPrintedMangleTarget(t, 2015, "(x => { let y = x; y?.z })()", "((x) => {\n let y = x;\n y == null || y.z;\n})();\n")

// Cannot substitute into call targets when it would change "this"
check("let x = arg0; x()", "arg0();")
check("let x = arg0; (0, x)()", "arg0();")
check("let x = arg0.foo; x.bar()", "arg0.foo.bar();")
check("let x = arg0.foo; x[bar]()", "arg0.foo[bar]();")
check("let x = arg0.foo; x()", "let x = arg0.foo;\nx();")
check("let x = arg0[foo]; x()", "let x = arg0[foo];\nx();")
check("let x = arg0?.foo; x()", "let x = arg0?.foo;\nx();")
check("let x = arg0?.[foo]; x()", "let x = arg0?.[foo];\nx();")
check("let x = arg0.foo; (0, x)()", "let x = arg0.foo;\nx();")
check("let x = arg0[foo]; (0, x)()", "let x = arg0[foo];\nx();")
check("let x = arg0?.foo; (0, x)()", "let x = arg0?.foo;\nx();")
check("let x = arg0?.[foo]; (0, x)()", "let x = arg0?.[foo];\nx();")
}

func TestTrimCodeInDeadControlFlow(t *testing.T) {
Expand Down
17 changes: 17 additions & 0 deletions scripts/end-to-end-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2616,6 +2616,23 @@
`,
}),
)

// Check variable initializer inlining
tests.push(
test(['in.js', '--outfile=node.js'].concat(minify), {
'in.js': `
function foo() {
if (this !== globalThis) throw 'fail'
}
function main() {
let obj = { bar: foo };
let fn = obj.bar;
(0, fn)();
}
main()
`,
}),
);
}

// Test minification of top-level symbols
Expand Down

0 comments on commit f3c6248

Please sign in to comment.