From f3c6248a6c64575bc60fbe9bfa537dbec4740fba Mon Sep 17 00:00:00 2001
From: Evan Wallace <evan.exe@gmail.com>
Date: Mon, 20 Jun 2022 18:36:11 -0400
Subject: [PATCH] fix minify bug with variable inlining and `this`

---
 CHANGELOG.md                         | 18 ++++++++++++++++++
 internal/js_parser/js_parser.go      |  9 +++++++++
 internal/js_parser/js_parser_test.go | 14 ++++++++++++++
 scripts/end-to-end-tests.js          | 17 +++++++++++++++++
 4 files changed, 58 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4911954d96f..7f2400f4891 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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))
diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
index f609b2e37b8..e7aa849b2f5 100644
--- a/internal/js_parser/js_parser.go
+++ b/internal/js_parser/js_parser.go
@@ -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
diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
index b3c706296b1..975a7a5ee86 100644
--- a/internal/js_parser/js_parser_test.go
+++ b/internal/js_parser/js_parser_test.go
@@ -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) {
diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js
index 3b176811942..1db1e3248b6 100644
--- a/scripts/end-to-end-tests.js
+++ b/scripts/end-to-end-tests.js
@@ -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