Drop superfluous __name() calls (#2062)

Author: indutny

CHANGELOG.md

@@ -56,6 +56,48 @@
 
     Some JavaScript environments such as Cloudflare Workers or Deno Deploy don't allow `new Function` because they disallow dynamic JavaScript evaluation. Previously esbuild's WebAssembly-based library used this to construct the WebAssembly worker function. With this release, the code is now inlined without using `new Function` so it will be able to run even when this restriction is in place.
 
+* Drop superfluous `__name()` calls ([#2062](https://github.com/evanw/esbuild/pull/2062))
+
+    When the `--keep-names` option is specified, esbuild inserts calls to a `__name` helper function to ensure that the `.name` property on function and class objects remains consistent even if the function or class name is renamed to avoid a name collision or because name minification is enabled. With this release, esbuild will now try to omit these calls to the `__name` helper function when the name of the function or class object was not renamed during the linking process after all:
+
+    ```js
+    // Original code
+    import { foo as foo1 } from 'data:text/javascript,export function foo() { return "foo1" }'
+    import { foo as foo2 } from 'data:text/javascript,export function foo() { return "foo2" }'
+    console.log(foo1.name, foo2.name)
+
+    // Old output (with --bundle --keep-names)
+    (() => {
+      var __defProp = Object.defineProperty;
+      var __name = (target, value) => __defProp(target, "name", { value, configurable: true });
+      function foo() {
+        return "foo1";
+      }
+      __name(foo, "foo");
+      function foo2() {
+        return "foo2";
+      }
+      __name(foo2, "foo");
+      console.log(foo.name, foo2.name);
+    })();
+
+    // New output (with --bundle --keep-names)
+    (() => {
+      var __defProp = Object.defineProperty;
+      var __name = (target, value) => __defProp(target, "name", { value, configurable: true });
+      function foo() {
+        return "foo1";
+      }
+      function foo2() {
+        return "foo2";
+      }
+      __name(foo2, "foo");
+      console.log(foo.name, foo2.name);
+    })();
+    ```
+
+    Notice how one of the calls to `__name` is now no longer printed. This change was contributed by [@indutny](https://github.com/indutny).
+
 ## 0.14.25
 
 * Reduce minification of CSS transforms to avoid Safari bugs ([#2057](https://github.com/evanw/esbuild/issues/2057))

internal/bundler/bundler_default_test.go

@@ -4358,6 +4358,71 @@ func TestDefineThis(t *testing.T) {
 	})
 }
 
+func TestKeepNamesWithNecessaryHelperFunctionCalls(t *testing.T) {
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import {
+					functionStmt as functionStmt1,
+					functionExpr as functionExpr1,
+					classStmt as classStmt1,
+					classExpr as classExpr1,
+					functionAnonExpr as functionAnonExpr1,
+					classAnonExpr as classAnonExpr1,
+				} from './copy1'
+
+				import {
+					functionStmt as functionStmt2,
+					functionExpr as functionExpr2,
+					classStmt as classStmt2,
+					classExpr as classExpr2,
+					functionAnonExpr as functionAnonExpr2,
+					classAnonExpr as classAnonExpr2,
+				} from './copy2'
+
+				console.log([
+					functionStmt1, functionStmt2,
+					functionExpr1, functionExpr2,
+					classStmt1, classStmt2,
+					classExpr1, classExpr2,
+					functionAnonExpr1, functionAnonExpr2,
+					classAnonExpr1, classAnonExpr2,
+				])
+			`,
+
+			"/copy1.js": `
+				export function functionStmt() { return 'copy1' }
+				export class classStmt { foo = 'copy1' }
+				export let functionExpr = function fn() { return 'copy1' }
+				export let classExpr = class cls { foo = 'copy1' }
+				export let functionAnonExpr = function() { return 'copy1' }
+				export let classAnonExpr = class { foo = 'copy1' }
+
+				class classStmtSideEffect { static [copy1]() {} }
+				let classExprSideEffect = class clsSideEffect { static [copy1]() {} }
+			`,
+
+			"/copy2.js": `
+				export function functionStmt() { return 'copy2' }
+				export class classStmt { foo = 'copy2' }
+				export let functionExpr = function fn() { return 'copy2' }
+				export let classExpr = class cls { foo = 'copy2' }
+				export let functionAnonExpr = function() { return 'copy2' }
+				export let classAnonExpr = class { foo = 'copy2' }
+
+				class classStmtSideEffect { static [copy2]() {} }
+				let classExprSideEffect = class clsSideEffect { static [copy2]() {} }
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+			KeepNames:     true,
+		},
+	})
+}
+
 func TestKeepNamesTreeShaking(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/snapshots/snapshots_default.txt

@@ -1401,19 +1401,95 @@ TestKeepNamesTreeShaking
 // entry.js
 function fnStmtKeep() {
 }
-__name(fnStmtKeep, "fnStmtKeep");
 x = fnStmtKeep;
 var fnExprKeep = /* @__PURE__ */ __name(function() {
 }, "keep");
 x = fnExprKeep;
 var clsStmtKeep = class {
 };
-__name(clsStmtKeep, "clsStmtKeep");
 new clsStmtKeep();
 var clsExprKeep = /* @__PURE__ */ __name(class {
 }, "keep");
 new clsExprKeep();
 
+================================================================================
+TestKeepNamesWithNecessaryHelperFunctionCalls
+---------- /out.js ----------
+// copy1.js
+function functionStmt() {
+  return "copy1";
+}
+var classStmt = class {
+  foo = "copy1";
+};
+var functionExpr = function fn() {
+  return "copy1";
+};
+var classExpr = class cls {
+  foo = "copy1";
+};
+var functionAnonExpr = function() {
+  return "copy1";
+};
+var classAnonExpr = class {
+  foo = "copy1";
+};
+var classStmtSideEffect = class {
+  static [copy1]() {
+  }
+};
+var classExprSideEffect = class clsSideEffect {
+  static [copy1]() {
+  }
+};
+
+// copy2.js
+function functionStmt2() {
+  return "copy2";
+}
+__name(functionStmt2, "functionStmt");
+var classStmt2 = class {
+  foo = "copy2";
+};
+__name(classStmt2, "classStmt");
+var functionExpr2 = /* @__PURE__ */ __name(function fn2() {
+  return "copy2";
+}, "fn");
+var classExpr2 = /* @__PURE__ */ __name(class cls2 {
+  foo = "copy2";
+}, "cls");
+var functionAnonExpr2 = /* @__PURE__ */ __name(function() {
+  return "copy2";
+}, "functionAnonExpr");
+var classAnonExpr2 = /* @__PURE__ */ __name(class {
+  foo = "copy2";
+}, "classAnonExpr");
+var classStmtSideEffect2 = class {
+  static [copy2]() {
+  }
+};
+__name(classStmtSideEffect2, "classStmtSideEffect");
+var classExprSideEffect2 = /* @__PURE__ */ __name(class clsSideEffect2 {
+  static [copy2]() {
+  }
+}, "clsSideEffect");
+
+// entry.js
+console.log([
+  functionStmt,
+  functionStmt2,
+  functionExpr,
+  functionExpr2,
+  classStmt,
+  classStmt2,
+  classExpr,
+  classExpr2,
+  functionAnonExpr,
+  functionAnonExpr2,
+  classAnonExpr,
+  classAnonExpr2
+]);
+
 ================================================================================
 TestLegalCommentsAvoidSlashTagEndOfFile
 ---------- /out/entry.js ----------
@@ -2810,12 +2886,11 @@ export function outer() {
     let inner = function() {
       return Math.random();
     };
-    __name(inner, "inner");
     const x = inner();
     console.log(x);
   }
 }
-__name(outer, "outer"), outer();
+outer();
 
 ================================================================================
 TestSwitchScopeNoBundle

internal/bundler/snapshots/snapshots_ts.txt

@@ -1102,7 +1102,6 @@ TestTypeScriptDecoratorsKeepNames
 // entry.ts
 var Foo = class {
 };
-__name(Foo, "Foo");
 Foo = __decorateClass([
   decoratorMustComeAfterName
 ], Foo);

internal/js_ast/js_ast.go

@@ -516,6 +516,8 @@ type ECall struct {
 	// call itself is removed due to this annotation, the arguments must remain
 	// if they have side effects.
 	CanBeUnwrappedIfUnused bool
+
+	IsKeepName bool
 }
 
 func (a *ECall) HasSameFlagsAs(b *ECall) bool {
@@ -848,11 +850,6 @@ type SLazyExport struct {
 
 type SExpr struct {
 	Value Expr
-
-	// This is set to true for automatically-generated expressions that should
-	// not affect tree shaking. For example, calling a function from the runtime
-	// that doesn't have externally-visible side effects.
-	DoesNotAffectTreeShaking bool
 }
 
 type EnumValue struct {

internal/js_parser/js_parser.go

@@ -7512,7 +7512,6 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt
 				prevStmt := result[len(result)-1]
 				if prevS, ok := prevStmt.Data.(*js_ast.SExpr); ok && !js_ast.IsSuperCall(prevStmt) && !js_ast.IsSuperCall(stmt) {
 					prevS.Value = js_ast.JoinWithComma(prevS.Value, s.Value)
-					prevS.DoesNotAffectTreeShaking = prevS.DoesNotAffectTreeShaking && s.DoesNotAffectTreeShaking
 					continue
 				}
 			}
@@ -8758,21 +8757,20 @@ func (p *parser) keepExprSymbolName(value js_ast.Expr, name string) js_ast.Expr
 
 	// Make sure tree shaking removes this if the function is never used
 	value.Data.(*js_ast.ECall).CanBeUnwrappedIfUnused = true
+	value.Data.(*js_ast.ECall).IsKeepName = true
 	return value
 }
 
 func (p *parser) keepStmtSymbolName(loc logger.Loc, ref js_ast.Ref, name string) js_ast.Stmt {
 	p.symbols[ref.InnerIndex].Flags |= js_ast.DidKeepName
 
-	return js_ast.Stmt{Loc: loc, Data: &js_ast.SExpr{
-		Value: p.callRuntime(loc, "__name", []js_ast.Expr{
-			{Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}},
-			{Loc: loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}},
-		}),
+	call := p.callRuntime(loc, "__name", []js_ast.Expr{
+		{Loc: loc, Data: &js_ast.EIdentifier{Ref: ref}},
+		{Loc: loc, Data: &js_ast.EString{Value: helpers.StringToUTF16(name)}},
+	})
+	call.Data.(*js_ast.ECall).IsKeepName = true
 
-		// Make sure tree shaking removes this if the function is never used
-		DoesNotAffectTreeShaking: true,
-	}}
+	return js_ast.Stmt{Loc: loc, Data: &js_ast.SExpr{Value: call}}
 }
 
 func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ast.Stmt {
@@ -14285,12 +14283,6 @@ func (p *parser) stmtsCanBeRemovedIfUnused(stmts []js_ast.Stmt) bool {
 			}
 
 		case *js_ast.SExpr:
-			if s.DoesNotAffectTreeShaking {
-				// Expressions marked with this are automatically generated and have
-				// no side effects by construction.
-				break
-			}
-
 			if !p.exprCanBeRemovedIfUnused(s.Value) {
 				return false
 			}
@@ -14464,7 +14456,7 @@ func (p *parser) exprCanBeRemovedIfUnused(expr js_ast.Expr) bool {
 		return true
 
 	case *js_ast.ECall:
-		canCallBeRemoved := e.CanBeUnwrappedIfUnused
+		canCallBeRemoved := e.CanBeUnwrappedIfUnused || e.IsKeepName
 
 		// Consider calls to our runtime "__publicField" function to be free of
 		// side effects for the purpose of expression removal. This allows class