fix #3125: bug with constant inlining and closures

CHANGELOG.md

@@ -4,7 +4,29 @@
 
 * Fix tree-shaking of classes with decorators ([#3164](https://github.com/evanw/esbuild/issues/3164))
 
-    This release fixes a bug where esbuild incorrectly allowed tree-shaking on classes with decorators. Each decorator is a function call, so classes with decorators must never be tree-shaken. This bug was a regression that was unintentionally introduced in version 0.18.2 by the change that enabled tree-shaking of lowered private fields.
+    This release fixes a bug where esbuild incorrectly allowed tree-shaking on classes with decorators. Each decorator is a function call, so classes with decorators must never be tree-shaken. This bug was a regression that was unintentionally introduced in version 0.18.2 by the change that enabled tree-shaking of lowered private fields. Previously decorators were always lowered, and esbuild always considered the automatically-generated decorator code to be a side effect. But this is no longer the case now that esbuild analyzes side effects using the AST before lowering takes place. This bug was fixed by considering any decorator a side effect.
+
+* Fix a minification bug involving function expressions ([#3125](https://github.com/evanw/esbuild/issues/3125))
+
+    When minification is enabled, esbuild does limited inlining of `const` symbols at the top of a scope. This release fixes a bug where inlineable symbols were incorrectly removed assuming that they were inlined. They may not be inlined in cases where they were referenced by earlier constants in the body of a function expression. The declarations involved in these edge cases are now kept instead of being removed:
+
+    ```js
+    // Original code
+    {
+      const fn = () => foo
+      const foo = 123
+      console.log(fn)
+    }
+
+    // Old output (with --minify-syntax)
+    console.log((() => foo)());
+
+    // New output (with --minify-syntax)
+    {
+      const fn = () => foo, foo = 123;
+      console.log(fn);
+    }
+    ```
 
 ## 0.18.5
 

internal/bundler_tests/bundler_dce_test.go

@@ -2849,6 +2849,13 @@ func TestConstValueInliningNoBundle(t *testing.T) {
 					)
 				}
 			`,
+			"/issue-3125.js": `
+				function foo() {
+					const f = () => x
+					const x = 0
+					return f()
+				}
+			`,
 		},
 		entryPaths: []string{
 			"/top-level.js",
@@ -2866,6 +2873,7 @@ func TestConstValueInliningNoBundle(t *testing.T) {
 			"/disabled-tdz.js",
 			"/backwards-reference-top-level.js",
 			"/backwards-reference-nested-function.js",
+			"/issue-3125.js",
 		},
 		options: config.Options{
 			Mode:         config.ModePassThrough,

internal/bundler_tests/snapshots/snapshots_dce.txt

@@ -250,6 +250,12 @@ function foo() {
   );
 }
 
+---------- /out/issue-3125.js ----------
+function foo() {
+  const f = () => x, x = 0;
+  return f();
+}
+
 ================================================================================
 TestCrossModuleConstantFolding
 ---------- /out/enum-entry.js ----------

internal/js_parser/js_parser.go

@@ -8592,7 +8592,7 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
 					end := 0
 					for _, d := range s.Decls {
 						if id, ok := d.Binding.Data.(*js_ast.BIdentifier); ok {
-							if _, ok := p.constValues[id.Ref]; ok {
+							if _, ok := p.constValues[id.Ref]; ok && p.symbols[id.Ref.InnerIndex].UseCountEstimate == 0 {
 								continue
 							}
 						}
@@ -10814,7 +10814,7 @@ func isUnsightlyPrimitive(data js_ast.E) bool {
 // But a situation like this is ok:
 //
 //	const x = 1;
-//	const y = [() => z];
+//	const y = [() => x + z];
 //	const z = 2;
 func isSafeForConstLocalPrefix(expr js_ast.Expr) bool {
 	switch e := expr.Data.(type) {

scripts/end-to-end-tests.js

@@ -2996,6 +2996,22 @@ for (const minify of [[], ['--minify-syntax']]) {
       `,
     }),
   );
+
+  // https://github.com/evanw/esbuild/issues/3125
+  tests.push(
+    test(['in.js', '--outfile=node.js'].concat(minify), {
+      'in.js': `
+        let y
+        {
+          // There was a bug where this incorrectly turned into "y = (() => x)()"
+          const f = () => x;
+          const x = 0;
+          y = f()
+        }
+        if (y !== 0) throw 'fail'
+      `,
+    }),
+  )
 }
 
 // Test minification of top-level symbols