fix #4028: minify live/dead switch cases better

evanw · Feb 7, 2025 · 7aa47c3 · 7aa47c3
1 parent 22ecd30
commit 7aa47c3
Show file tree

Hide file tree

Showing 5 changed files with 449 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -166,6 +166,29 @@
 
     The `--resolve-extensions=` option lets you specify the order in which to try resolving implicit file extensions. For complicated reasons, esbuild reorders TypeScript file extensions after JavaScript ones inside of `node_modules` so that JavaScript source code is always preferred to TypeScript source code inside of dependencies. However, this reordering had a bug that could accidentally change the relative order of TypeScript file extensions if one of them was a prefix of the other. That bug has been fixed in this release. You can see the issue for details.
 
+* Better minification of statically-determined `switch` cases ([#4028](https://github.com/evanw/esbuild/issues/4028))
+
+    With this release, esbuild will now try to trim unused code within `switch` statements when the test expression and `case` expressions are primitive literals. This can arise when the test expression is an identifier that is substituted for a primitive literal at compile time. For example:
+
+    ```js
+    // Original code
+    switch (MODE) {
+      case 'dev':
+        installDevToolsConsole()
+        break
+      case 'prod':
+        return
+      default:
+        throw new Error
+    }
+
+    // Old output (with --minify '--define:MODE="prod"')
+    switch("prod"){case"dev":installDevToolsConsole();break;case"prod":return;default:throw new Error}
+
+    // New output (with --minify '--define:MODE="prod"')
+    return;
+    ```
+
 * Emit `/* @__KEY__ */` for string literals derived from property names ([#4034](https://github.com/evanw/esbuild/issues/4034))
 
     Property name mangling is an advanced feature that shortens certain property names for better minification (I say "advanced feature" because it's very easy to break your code with it). Sometimes you need to store a property name in a string, such as `obj.get('foo')` instead of `obj.foo`. JavaScript minifiers such as esbuild and [Terser](https://terser.org/) have a convention where a `/* @__KEY__ */` comment before the string makes it behave like a property name. So `obj.get(/* @__KEY__ */ 'foo')` allows the contents of the string `'foo'` to be shortened.

diff --git a/internal/bundler_tests/bundler_dce_test.go b/internal/bundler_tests/bundler_dce_test.go
@@ -1430,6 +1430,89 @@ func TestDeadCodeInsideEmptyTry(t *testing.T) {
 	})
 }
 
+func TestDeadCodeInsideUnusedCases(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				// Unknown test value
+				switch (x) {
+					case 0: _ = require('./a'); break
+					case 1: _ = require('./b'); break
+				}
+
+				// Known test value
+				switch (1) {
+					case 0: _ = require('./FAIL-known-0'); break
+					case 1: _ = require('./a'); break
+					case 1: _ = require('./FAIL-known-1'); break
+					case 2: _ = require('./FAIL-known-2'); break
+				}
+
+				// Check for "default"
+				switch (1) {
+					case 1: _ = require('./a'); break
+					default: _ = require('./FAIL-default'); break
+				}
+				switch (0) {
+					case 1: _ = require('./FAIL-default-1'); break
+					default: _ = require('./a'); break
+					case 0: _ = require('./FAIL-default-0'); break
+				}
+
+				// Check for non-constant cases
+				switch (1) {
+					case x: _ = require('./a'); break
+					case 1: _ = require('./b'); break
+					case x: _ = require('./FAIL-x'); break
+					default: _ = require('./FAIL-x-default'); break
+				}
+
+				// Check for other kinds of jumps
+				for (const x of y)
+					switch (1) {
+						case 0: _ = require('./FAIL-continue-0'); continue
+						case 1: _ = require('./a'); continue
+						case 2: _ = require('./FAIL-continue-2'); continue
+					}
+				x = () => {
+					switch (1) {
+						case 0: _ = require('./FAIL-return-0'); return
+						case 1: _ = require('./a'); return
+						case 2: _ = require('./FAIL-return-2'); return
+					}
+				}
+
+				// Check for fall-through
+				switch ('b') {
+					case 'a': _ = require('./FAIL-fallthrough-a')
+					case 'b': _ = require('./a')
+					case 'c': _ = require('./b'); break
+					case 'd': _ = require('./FAIL-fallthrough-d')
+				}
+				switch ('b') {
+					case 'a': _ = require('./FAIL-fallthrough-a')
+					case 'b':
+					case 'c': _ = require('./a')
+					case 'd': _ = require('./b'); break
+					case 'e': _ = require('./FAIL-fallthrough-e')
+				}
+			`,
+			"/a.js": ``,
+			"/b.js": ``,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+		expectedScanLog: `entry.js: WARNING: This case clause will never be evaluated because it duplicates an earlier case clause
+entry.js: NOTE: The earlier case clause is here:
+entry.js: WARNING: This case clause will never be evaluated because it duplicates an earlier case clause
+entry.js: NOTE: The earlier case clause is here:
+`,
+	})
+}
+
 func TestRemoveTrailingReturn(t *testing.T) {
 	dce_suite.expectBundled(t, bundled{
 		files: map[string]string{

diff --git a/internal/bundler_tests/snapshots/snapshots_dce.txt b/internal/bundler_tests/snapshots/snapshots_dce.txt
@@ -930,6 +930,126 @@ try {
   require_d();
 }
 
+================================================================================
+TestDeadCodeInsideUnusedCases
+---------- /out.js ----------
+// a.js
+var require_a = __commonJS({
+  "a.js"() {
+  }
+});
+
+// b.js
+var require_b = __commonJS({
+  "b.js"() {
+  }
+});
+
+// entry.js
+switch (x) {
+  case 0:
+    _ = require_a();
+    break;
+  case 1:
+    _ = require_b();
+    break;
+}
+switch (1) {
+  case 0:
+    _ = null;
+    break;
+  case 1:
+    _ = require_a();
+    break;
+  case 1:
+    _ = null;
+    break;
+  case 2:
+    _ = null;
+    break;
+}
+switch (1) {
+  case 1:
+    _ = require_a();
+    break;
+  default:
+    _ = null;
+    break;
+}
+switch (0) {
+  case 1:
+    _ = null;
+    break;
+  default:
+    _ = require_a();
+    break;
+  case 0:
+    _ = null;
+    break;
+}
+switch (1) {
+  case x:
+    _ = require_a();
+    break;
+  case 1:
+    _ = require_b();
+    break;
+  case x:
+    _ = null;
+    break;
+  default:
+    _ = null;
+    break;
+}
+for (const x2 of y)
+  switch (1) {
+    case 0:
+      _ = null;
+      continue;
+    case 1:
+      _ = require_a();
+      continue;
+    case 2:
+      _ = null;
+      continue;
+  }
+x = () => {
+  switch (1) {
+    case 0:
+      _ = null;
+      return;
+    case 1:
+      _ = require_a();
+      return;
+    case 2:
+      _ = null;
+      return;
+  }
+};
+switch ("b") {
+  case "a":
+    _ = null;
+  case "b":
+    _ = require_a();
+  case "c":
+    _ = require_b();
+    break;
+  case "d":
+    _ = null;
+}
+switch ("b") {
+  case "a":
+    _ = null;
+  case "b":
+  case "c":
+    _ = require_a();
+  case "d":
+    _ = require_b();
+    break;
+  case "e":
+    _ = null;
+}
+
 ================================================================================
 TestDisableTreeShaking
 ---------- /out.js ----------