calling Symbol with a primitive will never throw

Author: evanw

CHANGELOG.md

@@ -1,5 +1,24 @@
 # Changelog
 
+## Unreleased
+
+* Allow tree-shaking of the `Symbol` constructor
+
+    With this release, calling `Symbol` is now considered to be side-effect free when the argument is known to be a primitive value. This means esbuild can now tree-shake module-level symbol variables:
+
+    ```js
+    // Original code
+    const a = Symbol('foo')
+    const b = Symbol(bar)
+
+    // Old output (with --tree-shaking=true)
+    const a = Symbol("foo");
+    const b = Symbol(bar);
+
+    // New output (with --tree-shaking=true)
+    const b = Symbol(bar);
+    ```
+
 ## 0.27.0
 
 **This release deliberately contains backwards-incompatible changes.** To avoid automatically picking up releases like this, you should either be pinning the exact version of `esbuild` in your `package.json` file (recommended) or be using a version range syntax that only accepts patch upgrades such as `^0.26.0` or `~0.26.0`. See npm's documentation about [semver](https://docs.npmjs.com/cli/v6/using-npm/semver/) for more information.

internal/bundler_tests/bundler_dce_test.go

@@ -4947,3 +4947,33 @@ func TestDCEOfIteratorSuperclassIssue4310(t *testing.T) {
 		},
 	})
 }
+
+func TestDCEOfSymbolCtorCall(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				const y0 = Symbol()
+				const y1 = Symbol(undefined)
+				const y2 = Symbol(null)
+				const y3 = Symbol(true)
+				const y4 = Symbol(123)
+				const y5 = Symbol(123n)
+				const y6 = Symbol('abc')
+				const y7 = Symbol(/* @__PURE__ */ (() => Math.random() < 0.5)() ? 'x' : 'y')
+
+				const n0 = Symbol({})
+				const n1 = Symbol(/./)
+				const n2 = Symbol(() => 0)
+				const n3 = Symbol(x)
+				const n4 = new Symbol('abc')
+				const n5 = Symbol(1, 2, 3)
+				const n6 = Symbol((() => Math.random() < 0.5)() ? 'x' : 'y')
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}

internal/bundler_tests/snapshots/snapshots_dce.txt

@@ -663,6 +663,18 @@ var Keep = class extends NotIterator {
 TestDCEOfNegatedBigints
 ---------- /out/entry.js ----------
 
+================================================================================
+TestDCEOfSymbolCtorCall
+---------- /out.js ----------
+// entry.js
+var n0 = Symbol({});
+var n1 = Symbol(/./);
+var n2 = Symbol(() => 0);
+var n3 = Symbol(x);
+var n4 = new Symbol("abc");
+var n5 = Symbol(1, 2, 3);
+var n6 = /* @__PURE__ */ Symbol((() => Math.random() < 0.5)() ? "x" : "y");
+
 ================================================================================
 TestDCEOfSymbolInstances
 ---------- /out/class.js ----------

internal/bundler_tests/snapshots/snapshots_ts.txt

@@ -118,7 +118,7 @@ console.log(a_exports, b_exports, c_exports, d_exports);
 ================================================================================
 TestTSAbstractClassFieldUseAssign
 ---------- /out.js ----------
-const keepThis = Symbol("keepThis");
+const keepThis = /* @__PURE__ */ Symbol("keepThis");
 keepThis;
 class Foo {
 }
@@ -127,7 +127,7 @@ class Foo {
 ================================================================================
 TestTSAbstractClassFieldUseDefine
 ---------- /out.js ----------
-const keepThisToo = Symbol("keepThisToo");
+const keepThisToo = /* @__PURE__ */ Symbol("keepThisToo");
 class Foo {
   keepThis;
   [keepThisToo];

internal/js_parser/js_parser.go

@@ -15135,63 +15135,74 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 				}
 			}
 
-			// Optimize references to global constructors
-			if p.options.minifySyntax && t.CanBeRemovedIfUnused && len(e.Args) <= 1 && !hasSpread {
+			// Handle certain special cases
+			if len(e.Args) <= 1 && !hasSpread {
 				if symbol := &p.symbols[t.Ref.InnerIndex]; symbol.Kind == ast.SymbolUnbound {
-					// Note: We construct expressions by assigning to "expr.Data" so
-					// that the source map position for the constructor is preserved
 					switch symbol.OriginalName {
-					case "Boolean":
-						if len(e.Args) == 0 {
-							return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EBoolean{Value: false}}, exprOut{}
-						} else {
-							expr.Data = &js_ast.EUnary{Value: p.astHelpers.SimplifyBooleanExpr(e.Args[0]), Op: js_ast.UnOpNot}
-							return js_ast.Not(expr), exprOut{}
+					case "Symbol":
+						// Calling the "Symbol()" constructor with a primitive will never throw
+						if len(e.Args) == 0 || js_ast.KnownPrimitiveType(e.Args[0].Data) != js_ast.PrimitiveUnknown {
+							e.CanBeUnwrappedIfUnused = true
 						}
+					}
 
-					case "Number":
-						if len(e.Args) == 0 {
-							return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ENumber{Value: 0}}, exprOut{}
-						} else {
-							arg := e.Args[0]
-
-							switch js_ast.KnownPrimitiveType(arg.Data) {
-							case js_ast.PrimitiveNumber:
-								return arg, exprOut{}
-
-							case
-								js_ast.PrimitiveUndefined, // NaN
-								js_ast.PrimitiveNull,      // 0
-								js_ast.PrimitiveBoolean,   // 0 or 1
-								js_ast.PrimitiveString:    // StringToNumber
-								if number, ok := js_ast.ToNumberWithoutSideEffects(arg.Data); ok {
-									expr.Data = &js_ast.ENumber{Value: number}
-								} else {
-									expr.Data = &js_ast.EUnary{Value: arg, Op: js_ast.UnOpPos}
+					// Optimize references to global constructors
+					if p.options.minifySyntax && t.CanBeRemovedIfUnused {
+						// Note: We construct expressions by assigning to "expr.Data" so
+						// that the source map position for the constructor is preserved
+						switch symbol.OriginalName {
+						case "Boolean":
+							if len(e.Args) == 0 {
+								return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EBoolean{Value: false}}, exprOut{}
+							} else {
+								expr.Data = &js_ast.EUnary{Value: p.astHelpers.SimplifyBooleanExpr(e.Args[0]), Op: js_ast.UnOpNot}
+								return js_ast.Not(expr), exprOut{}
+							}
+
+						case "Number":
+							if len(e.Args) == 0 {
+								return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.ENumber{Value: 0}}, exprOut{}
+							} else {
+								arg := e.Args[0]
+
+								switch js_ast.KnownPrimitiveType(arg.Data) {
+								case js_ast.PrimitiveNumber:
+									return arg, exprOut{}
+
+								case
+									js_ast.PrimitiveUndefined, // NaN
+									js_ast.PrimitiveNull,      // 0
+									js_ast.PrimitiveBoolean,   // 0 or 1
+									js_ast.PrimitiveString:    // StringToNumber
+									if number, ok := js_ast.ToNumberWithoutSideEffects(arg.Data); ok {
+										expr.Data = &js_ast.ENumber{Value: number}
+									} else {
+										expr.Data = &js_ast.EUnary{Value: arg, Op: js_ast.UnOpPos}
+									}
+									return expr, exprOut{}
 								}
-								return expr, exprOut{}
 							}
-						}
 
-					case "String":
-						if len(e.Args) == 0 {
-							return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EString{Value: nil}}, exprOut{}
-						} else {
-							arg := e.Args[0]
+						case "String":
+							if len(e.Args) == 0 {
+								return js_ast.Expr{Loc: expr.Loc, Data: &js_ast.EString{Value: nil}}, exprOut{}
+							} else {
+								arg := e.Args[0]
 
-							switch js_ast.KnownPrimitiveType(arg.Data) {
-							case js_ast.PrimitiveString:
-								return arg, exprOut{}
+								switch js_ast.KnownPrimitiveType(arg.Data) {
+								case js_ast.PrimitiveString:
+									return arg, exprOut{}
+								}
 							}
-						}
 
-					case "BigInt":
-						if len(e.Args) == 1 {
-							arg := e.Args[0]
+						case "BigInt":
+							if len(e.Args) == 1 {
+								arg := e.Args[0]
 
-							switch js_ast.KnownPrimitiveType(arg.Data) {
-							case js_ast.PrimitiveBigInt:
-								return arg, exprOut{}
+								switch js_ast.KnownPrimitiveType(arg.Data) {
+								case js_ast.PrimitiveBigInt:
+									return arg, exprOut{}
+								}
 							}
 						}
 					}