Skip to content

Commit

Permalink
fix #3161: panic when minifying static {}
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jun 15, 2023
1 parent a7a9096 commit 35fd509
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 19 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

* Fix a panic due to empty static class blocks ([#3161](https://github.com/evanw/esbuild/issues/3161))

This release fixes a bug where an internal invariant that was introduced in the previous release was sometimes violated, which then caused a panic. It happened when bundling code containing an empty static class block with both minification and bundling enabled.

## 0.18.2

* Lower static blocks when static fields are lowered ([#2800](https://github.com/evanw/esbuild/issues/2800), [#2950](https://github.com/evanw/esbuild/issues/2950), [#3025](https://github.com/evanw/esbuild/issues/3025))
Expand Down
46 changes: 46 additions & 0 deletions internal/bundler_tests/bundler_dce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2118,6 +2118,52 @@ func TestDCEClassStaticBlocks(t *testing.T) {
})
}

func TestDCEClassStaticBlocksMinifySyntax(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.ts": `
class A_REMOVE {
static {}
}
class B_REMOVE {
static { 123 }
}
class C_REMOVE {
static { /* @__PURE__*/ foo() }
}
class D_REMOVE {
static { try {} catch {} }
}
class E_REMOVE {
static { try { /* @__PURE__*/ foo() } catch {} }
}
class F_REMOVE {
static { try { 123 } catch { 123 } finally { 123 } }
}
class A_keep {
static { foo }
}
class B_keep {
static { this.foo }
}
class C_keep {
static { try { foo } catch {} }
}
class D_keep {
static { try {} finally { foo } }
}
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
MinifySyntax: true,
},
})
}

func TestDCEVarExports(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
27 changes: 27 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_dce.txt
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,33 @@ var D_keep = class {
}
})();

================================================================================
TestDCEClassStaticBlocksMinifySyntax
---------- /out.js ----------
// entry.ts
var A_keep = class {
};
foo;
var _B_keep = class {
}, B_keep = _B_keep;
_B_keep.foo;
var C_keep = class {
};
(() => {
try {
foo;
} catch {
}
})();
var D_keep = class {
};
(() => {
try {
} finally {
foo;
}
})();

================================================================================
TestDCEOfIIFE
---------- /out/remove-these.js ----------
Expand Down
18 changes: 0 additions & 18 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -10989,8 +10989,6 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul
// A scope is needed for private identifiers
p.pushScopeForVisitPass(js_ast.ScopeClassBody, class.BodyLoc)

end := 0

for i := range class.Properties {
property := &class.Properties[i]

Expand Down Expand Up @@ -11022,15 +11020,6 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul

p.fnOrArrowDataVisit = oldFnOrArrowData
p.fnOnlyDataVisit = oldFnOnlyDataVisit

// "class { static {} }" => "class {}"
if p.options.minifySyntax && len(property.ClassStaticBlock.Block.Stmts) == 0 {
continue
}

// Keep this property
class.Properties[end] = *property
end++
continue
}

Expand Down Expand Up @@ -11158,15 +11147,8 @@ func (p *parser) visitClass(nameScopeLoc logger.Loc, class *js_ast.Class, defaul

// Restore the ability to use "arguments" in decorators and computed properties
p.currentScope.ForbidArguments = false

// Keep this property
class.Properties[end] = *property
end++
}

// Finish the filtering operation
class.Properties = class.Properties[:end]

// Analyze side effects before adding the name keeping call
result.canBeRemovedIfUnused = js_ast.ClassCanBeRemovedIfUnused(*class, p.isUnbound)

Expand Down
7 changes: 6 additions & 1 deletion internal/js_parser/js_parser_lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -2047,7 +2047,7 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
}

if prop.Kind == js_ast.PropertyClassStaticBlock {
if p.options.unsupportedJSFeatures.Has(compat.ClassStaticBlocks) && len(prop.ClassStaticBlock.Block.Stmts) > 0 {
if p.options.unsupportedJSFeatures.Has(compat.ClassStaticBlocks) {
result.lowerAllStaticFields = true
}
continue
Expand Down Expand Up @@ -2306,6 +2306,11 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas

for _, prop := range class.Properties {
if prop.Kind == js_ast.PropertyClassStaticBlock {
// Drop empty class blocks when minifying
if p.options.minifySyntax && len(prop.ClassStaticBlock.Block.Stmts) == 0 {
continue
}

if classLoweringInfo.lowerAllStaticFields {
block := *prop.ClassStaticBlock
isAllExprs := []js_ast.Expr{}
Expand Down
12 changes: 12 additions & 0 deletions internal/js_parser/js_parser_lower_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,18 @@ func TestLowerClassStaticThis(t *testing.T) {
"var _a;\nx = (_a = class {\n}, __publicField(_a, \"x\", class extends _a {\n}), _a);\n")
}

func TestLowerClassStaticBlocks(t *testing.T) {
expectPrintedTarget(t, 2015, "class Foo { static {} }", "class Foo {\n}\n")
expectPrintedTarget(t, 2015, "class Foo { static {} x() {} }", "class Foo {\n x() {\n }\n}\n")
expectPrintedTarget(t, 2015, "class Foo { x() {} static {} }", "class Foo {\n x() {\n }\n}\n")
expectPrintedTarget(t, 2015, "class Foo { static { x } static {} static { y } }", "class Foo {\n}\nx;\ny;\n")

expectPrintedMangleTarget(t, 2015, "class Foo { static {} }", "class Foo {\n}\n")
expectPrintedMangleTarget(t, 2015, "class Foo { static {} x() {} }", "class Foo {\n x() {\n }\n}\n")
expectPrintedMangleTarget(t, 2015, "class Foo { x() {} static {} }", "class Foo {\n x() {\n }\n}\n")
expectPrintedMangleTarget(t, 2015, "class Foo { static { x } static {} static { y } }", "class Foo {\n}\nx, y;\n")
}

func TestLowerOptionalChain(t *testing.T) {
expectPrintedTarget(t, 2019, "a?.b.c", "a == null ? void 0 : a.b.c;\n")
expectPrintedTarget(t, 2019, "(a?.b).c", "(a == null ? void 0 : a.b).c;\n")
Expand Down
5 changes: 5 additions & 0 deletions internal/js_parser/js_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1935,6 +1935,11 @@ func TestClassStaticBlocks(t *testing.T) {
expectParseError(t, "class Foo { static { continue } }", "<stdin>: ERROR: Cannot use \"continue\" here:\n")
expectParseError(t, "x: { class Foo { static { break x } } }", "<stdin>: ERROR: There is no containing label named \"x\"\n")
expectParseError(t, "x: { class Foo { static { continue x } } }", "<stdin>: ERROR: There is no containing label named \"x\"\n")

expectPrintedMangle(t, "class Foo { static {} }", "class Foo {\n}\n")
expectPrintedMangle(t, "class Foo { static { 123 } }", "class Foo {\n}\n")
expectPrintedMangle(t, "class Foo { static { /* @__PURE__ */ foo() } }", "class Foo {\n}\n")
expectPrintedMangle(t, "class Foo { static { foo() } }", "class Foo {\n static {\n foo();\n }\n}\n")
}

func TestGenerator(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions scripts/js-api-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -6485,6 +6485,12 @@ class Foo {
assert.strictEqual(fromPromiseResolve(code4), `Promise.resolve().then(function(){return __toESM(require(foo))});\n`)
},

async classStaticBlocks({ esbuild }) {
// Check that empty static class blocks are still lowered (this was a regression in version 0.18.2)
assert.strictEqual((await esbuild.transform(`class Foo { static {} }`, { supported: { 'class-static-blocks': false } })).code, `class Foo {\n}\n`)
assert.strictEqual((await esbuild.transform(`class Foo { static { x } }`, { supported: { 'class-static-blocks': false } })).code, `class Foo {\n}\nx;\n`)
},

async inlineScript({ esbuild }) {
let p
assert.strictEqual((await esbuild.transform(`x = '</script>'`, {})).code, `x = "<\\/script>";\n`)
Expand Down

0 comments on commit 35fd509

Please sign in to comment.