From cbed653a039ab3e75e758301fcff3bd7eb3938d3 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Thu, 19 Dec 2024 13:27:33 -0500 Subject: [PATCH] fix #4003: minify empty `try`/`catch` blocks --- CHANGELOG.md | 17 +++++++ internal/bundler_tests/bundler_dce_test.go | 25 ++++++++++ .../bundler_tests/snapshots/snapshots_dce.txt | 40 ++++++++++++++-- internal/js_parser/js_parser.go | 47 +++++++++++++++++++ internal/js_parser/js_parser_test.go | 17 +++++++ 5 files changed, 142 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7938fcedcb1..5cbd9e29bfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,23 @@ The above code will be considered valid starting with this release. This change to esbuild follows a [similar change to TypeScript](https://github.com/microsoft/TypeScript/pull/60225) which will allow this syntax starting with TypeScript 5.7. +* Minify empty `try`/`catch`/`finally` blocks ([#4003](https://github.com/evanw/esbuild/issues/4003)) + + With this release, esbuild will now attempt to minify empty `try` blocks: + + ```js + // Original code + try {} catch { foo() } finally { bar() } + + // Old output (with --minify) + try{}catch{foo()}finally{bar()} + + // New output (with --minify) + bar(); + ``` + + This can sometimes expose additional minification opportunities. + ## 0.24.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.23.0` or `~0.23.0`. See npm's documentation about [semver](https://docs.npmjs.com/cli/v6/using-npm/semver/) for more information. diff --git a/internal/bundler_tests/bundler_dce_test.go b/internal/bundler_tests/bundler_dce_test.go index 27c71630e70..94531de3eb3 100644 --- a/internal/bundler_tests/bundler_dce_test.go +++ b/internal/bundler_tests/bundler_dce_test.go @@ -1405,6 +1405,31 @@ func TestDeadCodeFollowingJump(t *testing.T) { }) } +func TestDeadCodeInsideEmptyTry(t *testing.T) { + dce_suite.expectBundled(t, bundled{ + files: map[string]string{ + "/entry.js": ` + try { foo() } + catch { require('./a') } + finally { require('./b') } + + try {} + catch { require('./c') } + finally { require('./d') } + `, + "/a.js": ``, + "/b.js": ``, + "/c.js": `TEST FAILED`, // Dead code paths should not import code + "/d.js": ``, + }, + entryPaths: []string{"/entry.js"}, + options: config.Options{ + Mode: config.ModeBundle, + AbsOutputFile: "/out.js", + }, + }) +} + 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 index 89dba531d75..d4320f1f7f2 100644 --- a/internal/bundler_tests/snapshots/snapshots_dce.txt +++ b/internal/bundler_tests/snapshots/snapshots_dce.txt @@ -458,10 +458,7 @@ var A_keep = class { } }, D_keep = class { static { - try { - } finally { - foo; - } + foo; } }; @@ -892,6 +889,41 @@ testBreak(); testContinue(); testStmts(); +================================================================================ +TestDeadCodeInsideEmptyTry +---------- /out.js ---------- +// a.js +var require_a = __commonJS({ + "a.js"() { + } +}); + +// b.js +var require_b = __commonJS({ + "b.js"() { + } +}); + +// d.js +var require_d = __commonJS({ + "d.js"() { + } +}); + +// entry.js +try { + foo(); +} catch { + require_a(); +} finally { + require_b(); +} +try { +} catch { +} finally { + require_d(); +} + ================================================================================ TestDisableTreeShaking ---------- /out.js ---------- diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 444cc144ef2..d60894f05d9 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -10774,6 +10774,13 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ p.popScope() if s.Catch != nil { + old := p.isControlFlowDead + + // If the try body is empty, then the catch body is dead + if len(s.Block.Stmts) == 0 { + p.isControlFlowDead = true + } + p.pushScopeForVisitPass(js_ast.ScopeCatchBinding, s.Catch.Loc) if s.Catch.BindingOrNil.Data != nil { p.visitBinding(s.Catch.BindingOrNil, bindingOpts{}) @@ -10785,6 +10792,8 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ p.lowerObjectRestInCatchBinding(s.Catch) p.popScope() + + p.isControlFlowDead = old } if s.Finally != nil { @@ -10793,6 +10802,44 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_ p.popScope() } + // Drop the whole thing if the try body is empty + if p.options.minifySyntax && len(s.Block.Stmts) == 0 { + keepCatch := false + + // Certain "catch" blocks need to be preserved: + // + // try {} catch { let foo } // Can be removed + // try {} catch { var foo } // Must be kept + // + if s.Catch != nil { + for _, stmt2 := range s.Catch.Block.Stmts { + if shouldKeepStmtInDeadControlFlow(stmt2) { + keepCatch = true + break + } + } + } + + // Make sure to preserve the "finally" block if present + if !keepCatch { + if s.Finally == nil { + return stmts + } + finallyNeedsBlock := false + for _, stmt2 := range s.Finally.Block.Stmts { + if statementCaresAboutScope(stmt2) { + finallyNeedsBlock = true + break + } + } + if !finallyNeedsBlock { + return append(stmts, s.Finally.Block.Stmts...) + } + block := s.Finally.Block + stmt = js_ast.Stmt{Loc: s.Finally.Loc, Data: &block} + } + } + case *js_ast.SSwitch: s.Test = p.visitExpr(s.Test) p.pushScopeForVisitPass(js_ast.ScopeBlock, s.BodyLoc) diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index 732ed029f6a..eb08d10e798 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -6548,6 +6548,23 @@ func TestMangleCatch(t *testing.T) { expectPrintedMangle(t, "if (y) try { throw 1 } catch (x) {} else eval('x')", "if (y) try {\n throw 1;\n} catch {\n}\nelse eval(\"x\");\n") } +func TestMangleEmptyTry(t *testing.T) { + expectPrintedMangle(t, "try { throw 0 } catch (e) { foo() }", "try {\n throw 0;\n} catch {\n foo();\n}\n") + expectPrintedMangle(t, "try {} catch (e) { var foo }", "try {\n} catch {\n var foo;\n}\n") + + expectPrintedMangle(t, "try {} catch (e) { foo() }", "") + expectPrintedMangle(t, "try {} catch (e) { foo() } finally {}", "") + + expectPrintedMangle(t, "try {} finally { foo() }", "foo();\n") + expectPrintedMangle(t, "try {} catch (e) { foo() } finally { bar() }", "bar();\n") + + expectPrintedMangle(t, "try {} finally { var x = foo() }", "var x = foo();\n") + expectPrintedMangle(t, "try {} catch (e) { foo() } finally { var x = bar() }", "var x = bar();\n") + + expectPrintedMangle(t, "try {} finally { let x = foo() }", "{\n let x = foo();\n}\n") + expectPrintedMangle(t, "try {} catch (e) { foo() } finally { let x = bar() }", "{\n let x = bar();\n}\n") +} + func TestAutoPureForObjectCreate(t *testing.T) { expectPrinted(t, "Object.create(null)", "/* @__PURE__ */ Object.create(null);\n") expectPrinted(t, "Object.create({})", "/* @__PURE__ */ Object.create({});\n")