fix #282: minification of property function calls

evanw · Jul 24, 2020 · 95cff1e · 95cff1e
1 parent 2abe024
commit 95cff1e
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+* Fix minification bug with `this` values for function calls ([#282](https://github.com/evanw/esbuild/issues/282))
+
+    Previously `(0, this.fn)()` was incorrectly minified to `this.fn()`, which changes the value of `this` used for the function call. Now syntax like this is preserved during minification.
+
 ## 0.6.5
 
 * Fix IIFE wrapper for ES5

diff --git a/internal/parser/parser.go b/internal/parser/parser.go
@@ -341,6 +341,16 @@ func checkEqualityIfNoSideEffects(left ast.E, right ast.E) (bool, bool) {
 	return false, false
 }
 
+func hasValueForThisInCall(expr ast.Expr) bool {
+	switch expr.Data.(type) {
+	case *ast.EDot, *ast.EIndex:
+		return true
+
+	default:
+		return false
+	}
+}
+
 func (p *parser) pushScopeForParsePass(kind ast.ScopeKind, loc ast.Loc) int {
 	parent := p.currentScope
 	scope := &ast.Scope{
@@ -6920,6 +6930,7 @@ func (p *parser) visitExprInOut(expr ast.Expr, in exprIn) (ast.Expr, exprOut) {
 		}
 
 	case *ast.EBinary:
+		isCallTarget := e == p.callTarget
 		e.Left, _ = p.visitExprInOut(e.Left, exprIn{assignTarget: e.Op.BinaryAssignTarget()})
 
 		// Pattern-match "typeof require == 'function' && ___" from browserify
@@ -6937,6 +6948,12 @@ func (p *parser) visitExprInOut(expr ast.Expr, in exprIn) (ast.Expr, exprOut) {
 			if p.MangleSyntax {
 				e.Left = p.simplifyUnusedExpr(e.Left)
 				if e.Left.Data == nil {
+					// "(1, fn)()" => "fn()"
+					// "(1, this.fn)" => "this.fn"
+					// "(1, this.fn)()" => "(0, this.fn)()"
+					if isCallTarget && hasValueForThisInCall(e.Right) {
+						return ast.JoinWithComma(ast.Expr{Loc: e.Left.Loc, Data: &ast.ENumber{}}, e.Right), exprOut{}
+					}
 					return e.Right, exprOut{}
 				}
 			}
@@ -6986,6 +7003,12 @@ func (p *parser) visitExprInOut(expr ast.Expr, in exprIn) (ast.Expr, exprOut) {
 				return e.Left, exprOut{}
 
 			case *ast.ENull, *ast.EUndefined:
+				// "(null ?? fn)()" => "fn()"
+				// "(null ?? this.fn)" => "this.fn"
+				// "(null ?? this.fn)()" => "(0, this.fn)()"
+				if isCallTarget && hasValueForThisInCall(e.Right) {
+					return ast.JoinWithComma(ast.Expr{Loc: e.Left.Loc, Data: &ast.ENumber{}}, e.Right), exprOut{}
+				}
 				return e.Right, exprOut{}
 
 			default:
@@ -6999,13 +7022,25 @@ func (p *parser) visitExprInOut(expr ast.Expr, in exprIn) (ast.Expr, exprOut) {
 				if boolean {
 					return e.Left, exprOut{}
 				} else {
+					// "(0 || fn)()" => "fn()"
+					// "(0 || this.fn)" => "this.fn"
+					// "(0 || this.fn)()" => "(0, this.fn)()"
+					if isCallTarget && hasValueForThisInCall(e.Right) {
+						return ast.JoinWithComma(ast.Expr{Loc: e.Left.Loc, Data: &ast.ENumber{}}, e.Right), exprOut{}
+					}
 					return e.Right, exprOut{}
 				}
 			}
 
 		case ast.BinOpLogicalAnd:
 			if boolean, ok := toBooleanWithoutSideEffects(e.Left.Data); ok {
 				if boolean {
+					// "(1 && fn)()" => "fn()"
+					// "(1 && this.fn)" => "this.fn"
+					// "(1 && this.fn)()" => "(0, this.fn)()"
+					if isCallTarget && hasValueForThisInCall(e.Right) {
+						return ast.JoinWithComma(ast.Expr{Loc: e.Left.Loc, Data: &ast.ENumber{}}, e.Right), exprOut{}
+					}
 					return e.Right, exprOut{}
 				} else {
 					return e.Left, exprOut{}
@@ -7415,15 +7450,28 @@ func (p *parser) visitExprInOut(expr ast.Expr, in exprIn) (ast.Expr, exprOut) {
 		}
 
 	case *ast.EIf:
+		isCallTarget := e == p.callTarget
 		e.Test = p.visitBooleanExpr(e.Test)
 		e.Yes = p.visitExpr(e.Yes)
 		e.No = p.visitExpr(e.No)
 
 		// Fold constants
 		if boolean, ok := toBooleanWithoutSideEffects(e.Test.Data); ok {
 			if boolean {
+				// "(1 ? fn : 2)()" => "fn()"
+				// "(1 ? this.fn : 2)" => "this.fn"
+				// "(1 ? this.fn : 2)()" => "(0, this.fn)()"
+				if isCallTarget && hasValueForThisInCall(e.Yes) {
+					return ast.JoinWithComma(ast.Expr{Loc: e.Test.Loc, Data: &ast.ENumber{}}, e.Yes), exprOut{}
+				}
 				return e.Yes, exprOut{}
 			} else {
+				// "(0 ? 1 : fn)()" => "fn()"
+				// "(0 ? 1 : this.fn)" => "this.fn"
+				// "(0 ? 1 : this.fn)()" => "(0, this.fn)()"
+				if isCallTarget && hasValueForThisInCall(e.No) {
+					return ast.JoinWithComma(ast.Expr{Loc: e.Test.Loc, Data: &ast.ENumber{}}, e.No), exprOut{}
+				}
 				return e.No, exprOut{}
 			}
 		}

diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go
@@ -1377,6 +1377,31 @@ func TestMangleUnused(t *testing.T) {
 	expectPrintedMangle(t, "let x = (y, 2)", "let x = (y, 2);\n")
 	expectPrintedMangle(t, "let x = (/* @__PURE__ */ foo(bar), 2)", "let x = (bar, 2);\n")
 
+	expectPrintedMangle(t, "let x = (2, y)", "let x = y;\n")
+	expectPrintedMangle(t, "let x = (2, y)()", "let x = y();\n")
+	expectPrintedMangle(t, "let x = (true && y)()", "let x = y();\n")
+	expectPrintedMangle(t, "let x = (false || y)()", "let x = y();\n")
+	expectPrintedMangle(t, "let x = (null ?? y)()", "let x = y();\n")
+	expectPrintedMangle(t, "let x = (1 ? y : 2)()", "let x = y();\n")
+	expectPrintedMangle(t, "let x = (0 ? 1 : y)()", "let x = y();\n")
+
+	// Make sure call targets with "this" values are preserved
+	expectPrintedMangle(t, "let x = (2, y.z)", "let x = y.z;\n")
+	expectPrintedMangle(t, "let x = (2, y.z)()", "let x = (0, y.z)();\n")
+	expectPrintedMangle(t, "let x = (true && y.z)()", "let x = (0, y.z)();\n")
+	expectPrintedMangle(t, "let x = (false || y.z)()", "let x = (0, y.z)();\n")
+	expectPrintedMangle(t, "let x = (null ?? y.z)()", "let x = (0, y.z)();\n")
+	expectPrintedMangle(t, "let x = (1 ? y.z : 2)()", "let x = (0, y.z)();\n")
+	expectPrintedMangle(t, "let x = (0 ? 1 : y.z)()", "let x = (0, y.z)();\n")
+
+	expectPrintedMangle(t, "let x = (2, y[z])", "let x = y[z];\n")
+	expectPrintedMangle(t, "let x = (2, y[z])()", "let x = (0, y[z])();\n")
+	expectPrintedMangle(t, "let x = (true && y[z])()", "let x = (0, y[z])();\n")
+	expectPrintedMangle(t, "let x = (false || y[z])()", "let x = (0, y[z])();\n")
+	expectPrintedMangle(t, "let x = (null ?? y[z])()", "let x = (0, y[z])();\n")
+	expectPrintedMangle(t, "let x = (1 ? y[z] : 2)()", "let x = (0, y[z])();\n")
+	expectPrintedMangle(t, "let x = (0 ? 1 : y[z])()", "let x = (0, y[z])();\n")
+
 	expectPrintedMangle(t, "foo ? 1 : 2", "foo;\n")
 	expectPrintedMangle(t, "foo ? 1 : bar", "foo || bar;\n")
 	expectPrintedMangle(t, "foo ? bar : 2", "foo && bar;\n")