Skip to content

Commit

Permalink
fix #2800, fix #2950, fix #3025: static blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Jun 12, 2023
1 parent a1a1e44 commit c12cc90
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 85 deletions.
33 changes: 33 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,38 @@
# Changelog

## Unreleased

* 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))

This release fixes a bug where esbuild incorrectly did not lower static class blocks when static class fields needed to be lowered. For example, the following code should print `1 2 3` but previously printed `2 1 3` instead due to this bug:

```js
// Original code
class Foo {
static x = console.log(1)
static { console.log(2) }
static y = console.log(3)
}

// Old output (with --supported:class-static-field=false)
class Foo {
static {
console.log(2);
}
}
__publicField(Foo, "x", console.log(1));
__publicField(Foo, "y", console.log(3));

// New output (with --supported:class-static-field=false)
class Foo {
}
__publicField(Foo, "x", console.log(1));
(() => {
console.log(2);
})();
__publicField(Foo, "y", console.log(3));
```

## 0.18.1

* Fill in `null` entries in input source maps ([#3144](https://github.com/evanw/esbuild/issues/3144))
Expand Down
39 changes: 20 additions & 19 deletions internal/bundler_tests/snapshots/snapshots_dce.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,31 +340,32 @@ TestDCEClassStaticBlocks
---------- /out.js ----------
// entry.ts
var A_keep = class {
static {
foo;
}
};
var B_keep = class {
static {
this.foo;
}
(() => {
foo;
})();
var _B_keep = class {
};
var B_keep = _B_keep;
(() => {
_B_keep.foo;
})();
var C_keep = class {
static {
try {
foo;
} catch {
}
}
};
var D_keep = class {
static {
try {
} finally {
foo;
}
(() => {
try {
foo;
} catch {
}
})();
var D_keep = class {
};
(() => {
try {
} finally {
foo;
}
})();

================================================================================
TestDCEOfIIFE
Expand Down
33 changes: 15 additions & 18 deletions internal/bundler_tests/snapshots/snapshots_lower.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2373,25 +2373,22 @@ var B = (_a = class {
TestStaticClassBlockESNext
---------- /out.js ----------
// entry.js
var A = class {
static {
}
static {
this.thisField++;
A.classField++;
super.superField = super.superField + 1;
super.superField++;
}
};
var B = class {
static {
}
static {
this.thisField++;
super.superField = super.superField + 1;
super.superField++;
}
var _A = class {
};
var A = _A;
(() => {
_A.thisField++;
_A.classField++;
__superSet(_A, _A, "superField", __superGet(_A, _A, "superField") + 1);
__superWrapper(_A, _A, "superField")._++;
})();
var _a;
var B = (_a = class {
}, (() => {
_a.thisField++;
__superSet(_a, _a, "superField", __superGet(_a, _a, "superField") + 1);
__superWrapper(_a, _a, "superField")._++;
})(), _a);

================================================================================
TestTSLowerClassField2020NoBundle
Expand Down
83 changes: 35 additions & 48 deletions internal/js_parser/js_parser_lower.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2032,6 +2032,40 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
// _foo = new WeakMap();
//
for _, prop := range class.Properties {
// Be conservative and always lower static fields when we're doing TDZ-
// avoidance if the class's shadowing symbol is referenced at all (i.e.
// the class name within the class body, which can be referenced by name
// or by "this" in a static initializer). We can't transform this:
//
// class Foo {
// static foo = new Foo();
// static #bar = new Foo();
// static { new Foo(); }
// }
//
// into this:
//
// var Foo = class {
// static foo = new Foo();
// static #bar = new Foo();
// static { new Foo(); }
// };
//
// since "new Foo" will crash. We need to lower this static field to avoid
// crashing due to an uninitialized binding.
if result.avoidTDZ {
// Note that due to esbuild's single-pass design where private fields
// are lowered as they are resolved, we must decide whether to lower
// these private fields before we enter the class body. We can't wait
// until we've scanned the class body and know if the shadowing symbol
// is used or not before we decide, because if "#bar" does need to be
// lowered, references to "#bar" inside the class body weren't lowered.
// So we just unconditionally do this instead.
if prop.Kind == js_ast.PropertyClassStaticBlock || prop.Flags.Has(js_ast.PropertyIsStatic) {
result.lowerAllStaticFields = true
}
}

if prop.Kind == js_ast.PropertyClassStaticBlock {
if p.options.unsupportedJSFeatures.Has(compat.ClassStaticBlocks) && len(prop.ClassStaticBlock.Block.Stmts) > 0 {
result.lowerAllStaticFields = true
Expand All @@ -2044,34 +2078,6 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
if p.privateSymbolNeedsToBeLowered(private) {
result.lowerAllStaticFields = true
}

// Be conservative and always lower static fields when we're doing TDZ-
// avoidance if the class's shadowing symbol is referenced at all (i.e.
// the class name within the class body, which can be referenced by name
// or by "this" in a static initializer). We can't transform this:
//
// class Foo {
// static #foo = new Foo();
// }
//
// into this:
//
// var Foo = class {
// static #foo = new Foo();
// };
//
// since "new Foo" will crash. We need to lower this static field to avoid
// crashing due to an uninitialized binding.
if result.avoidTDZ {
// Note that due to esbuild's single-pass design where private fields
// are lowered as they are resolved, we must decide whether to lower
// these private fields before we enter the class body. We can't wait
// until we've scanned the class body and know if the shadowing symbol
// is used or not before we decide, because if "#foo" does need to be
// lowered, references to "#foo" inside the class body weren't lowered.
// So we just unconditionally do this instead.
result.lowerAllStaticFields = true
}
} else {
if p.privateSymbolNeedsToBeLowered(private) {
result.lowerAllInstanceFields = true
Expand Down Expand Up @@ -2161,25 +2167,6 @@ func (p *parser) computeClassLoweringInfo(class *js_ast.Class) (result classLowe
if p.options.ts.Parse && !result.useDefineForClassFields {
result.lowerAllStaticFields = true
}

// Be conservative and always lower static fields when we're doing TDZ-
// avoidance. We can't transform this:
//
// class Foo {
// static foo = new Foo();
// }
//
// into this:
//
// var Foo = class {
// static foo = new Foo();
// };
//
// since "new Foo" will crash. We need to lower this static field to avoid
// crashing due to an uninitialized binding.
if result.avoidTDZ {
result.lowerAllStaticFields = true
}
} else {
// Instance fields must be lowered if the target doesn't support them
if p.options.unsupportedJSFeatures.Has(compat.ClassField) {
Expand Down Expand Up @@ -2357,7 +2344,7 @@ 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 {
if p.options.unsupportedJSFeatures.Has(compat.ClassStaticBlocks) {
if classLoweringInfo.lowerAllStaticFields {
if block := *prop.ClassStaticBlock; len(block.Block.Stmts) > 0 {
staticMembers = append(staticMembers, js_ast.Expr{Loc: prop.Loc, Data: &js_ast.ECall{
Target: js_ast.Expr{Loc: prop.Loc, Data: &js_ast.EArrow{Body: js_ast.FnBody{
Expand Down
53 changes: 53 additions & 0 deletions scripts/end-to-end-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4541,6 +4541,59 @@ for (let flags of [[], ['--target=es6']]) {
)
}

// Class lowering tests with bundling
for (let flags of [[], ['--target=es6'], ['--bundle'], ['--bundle', '--target=es6']]) {
tests.push(
test(['in.js', '--outfile=node.js'].concat(flags), {
'in.js': `
const order = []
class Test {
static first = order.push(1)
static { order.push(2) }
static third = order.push(3)
}
if ('' + order !== '1,2,3') throw 'fail: ' + order
`,
}),

// https://github.com/evanw/esbuild/issues/2800
test(['in.js', '--outfile=node.js'].concat(flags), {
'in.js': `
class Baz {
static thing = "value"
static {
this.prototype.thing = "value"
}
}
if (new Baz().thing !== 'value') throw 'fail'
`,
}),

// https://github.com/evanw/esbuild/issues/2950
test(['in.js', '--outfile=node.js'].concat(flags), {
'in.js': `
class SomeClass {
static { this.One = 1; }
static { this.Two = SomeClass.One * 2; }
}
if (SomeClass.Two !== 2) throw 'fail'
`,
}),

// https://github.com/evanw/esbuild/issues/3025
test(['in.js', '--outfile=node.js'].concat(flags), {
'in.js': `
class Foo {
static {
Foo.prototype.foo = 'foo'
}
}
if (new Foo().foo !== 'foo') throw 'fail'
`,
}),
)
}

// Async lowering tests
for (let flags of [[], ['--target=es2017'], ['--target=es6']]) {
tests.push(
Expand Down

0 comments on commit c12cc90

Please sign in to comment.