Skip to content

Commit

Permalink
fix #3684: abstract experimental decorators
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Mar 14, 2024
1 parent c809af0 commit ae5cc17
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 18 deletions.
30 changes: 30 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,36 @@

## Unreleased

* Support TypeScript experimental decorators on `abstract` class fields ([#3684](https://github.com/evanw/esbuild/issues/3684))

With this release, you can now use TypeScript experimental decorators on `abstract` class fields. This was silently compiled incorrectly in esbuild 0.19.7 and below, and was an error from esbuild 0.19.8 to esbuild 0.20.1. Code such as the following should now work correctly:

```ts
// Original code
const log = (x: any, y: string) => console.log(y)
abstract class Foo { @log abstract foo: string }
new class extends Foo { foo = '' }

// Old output (with --loader=ts --tsconfig-raw={\"compilerOptions\":{\"experimentalDecorators\":true}})
const log = (x, y) => console.log(y);
class Foo {
}
new class extends Foo {
foo = "";
}();

// New output (with --loader=ts --tsconfig-raw={\"compilerOptions\":{\"experimentalDecorators\":true}})
const log = (x, y) => console.log(y);
class Foo {
}
__decorateClass([
log
], Foo.prototype, "foo", 2);
new class extends Foo {
foo = "";
}();
```

* Improve dead code removal of `switch` statements ([#3659](https://github.com/evanw/esbuild/issues/3659))

With this release, esbuild will now remove `switch` statements in branches when minifying if they are known to never be evaluated:
Expand Down
2 changes: 2 additions & 0 deletions internal/bundler_tests/bundler_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,7 @@ func TestTSExperimentalDecorators(t *testing.T) {
@x @y mDef = 1
@x @y method(@x0 @y0 arg0, @x1 @y1 arg1) { return new Foo }
@x @y declare mDecl
@x @y abstract mAbst
constructor(@x0 @y0 arg0, @x1 @y1 arg1) {}
@x @y static sUndef
Expand All @@ -1070,6 +1071,7 @@ func TestTSExperimentalDecorators(t *testing.T) {
@x @y [mDef()] = 1
@x @y [method()](@x0 @y0 arg0, @x1 @y1 arg1) { return new Foo }
@x @y declare [mDecl()]
@x @y abstract [mAbst()]
// Side effect order must be preserved even for fields without decorators
[xUndef()]
Expand Down
26 changes: 17 additions & 9 deletions internal/bundler_tests/snapshots/snapshots_ts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,10 @@ __decorateClass([
x,
y
], Foo.prototype, "mDecl", 2);
__decorateClass([
x,
y
], Foo.prototype, "mAbst", 2);
__decorateClass([
x,
y
Expand Down Expand Up @@ -747,24 +751,24 @@ Foo = __decorateClass([
], Foo);

// all_computed.ts
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j;
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k;
var Foo2 = class {
constructor() {
this[_b] = 1;
this[_e] = 2;
this[_f] = 2;
}
static {
_j = mDecl();
_k = mDecl();
}
[(_a = mUndef(), _b = mDef(), _c = method())](arg0, arg1) {
return new Foo2();
}
static [(_d = mDecl(), xUndef(), _e = xDef(), yUndef(), _f = yDef(), _g = sUndef(), _h = sDef(), _i = sMethod())](arg0, arg1) {
static [(_d = mDecl(), _e = mAbst(), xUndef(), _f = xDef(), yUndef(), _g = yDef(), _h = sUndef(), _i = sDef(), _j = sMethod())](arg0, arg1) {
return new Foo2();
}
};
Foo2[_f] = 3;
Foo2[_h] = new Foo2();
Foo2[_g] = 3;
Foo2[_i] = new Foo2();
__decorateClass([
x,
y
Expand All @@ -788,23 +792,27 @@ __decorateClass([
__decorateClass([
x,
y
], Foo2, _g, 2);
], Foo2.prototype, _e, 2);
__decorateClass([
x,
y
], Foo2, _h, 2);
__decorateClass([
x,
y
], Foo2, _i, 2);
__decorateClass([
x,
y,
__decorateParam(0, x0),
__decorateParam(0, y0),
__decorateParam(1, x1),
__decorateParam(1, y1)
], Foo2, _i, 1);
], Foo2, _j, 1);
__decorateClass([
x,
y
], Foo2, _j, 2);
], Foo2, _k, 2);
Foo2 = __decorateClass([
x?.[_ + "y"](),
new y?.[_ + "x"]()
Expand Down
2 changes: 1 addition & 1 deletion internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ const (
PropertySet
PropertyAutoAccessor
PropertySpread
PropertyDeclare
PropertyDeclareOrAbstract
PropertyClassStaticBlock
)

Expand Down
30 changes: 28 additions & 2 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2204,7 +2204,7 @@ func (p *parser) parseProperty(startLoc logger.Loc, kind js_ast.PropertyKind, op
// https://github.com/evanw/esbuild/issues/1675
// https://github.com/microsoft/TypeScript/issues/46345
//
prop.Kind = js_ast.PropertyDeclare
prop.Kind = js_ast.PropertyDeclareOrAbstract
return prop, true
}

Expand All @@ -2216,7 +2216,33 @@ func (p *parser) parseProperty(startLoc logger.Loc, kind js_ast.PropertyKind, op
if !p.lexer.HasNewlineBefore && opts.isClass && p.options.ts.Parse && !opts.isTSAbstract && raw == name.String {
opts.isTSAbstract = true
scopeIndex := len(p.scopesInOrder)
p.parseProperty(startLoc, kind, opts, nil)

if prop, ok := p.parseProperty(startLoc, kind, opts, nil); ok &&
prop.Kind == js_ast.PropertyNormal && prop.ValueOrNil.Data == nil &&
(p.options.ts.Config.ExperimentalDecorators == config.True && len(opts.decorators) > 0) {
// If this is a well-formed class field with the "abstract" keyword,
// only keep the declaration to preserve its side-effects when
// there are TypeScript experimental decorators present:
//
// abstract class Foo {
// // Remove this
// abstract [(console.log('side effect 1'), 'foo')]
//
// // Keep this
// @decorator(console.log('side effect 2')) abstract bar
// }
//
// This behavior is valid with TypeScript experimental decorators.
// TypeScript does not allow this with JavaScript decorators.
//
// References:
//
// https://github.com/evanw/esbuild/issues/3684
//
prop.Kind = js_ast.PropertyDeclareOrAbstract
return prop, true
}

p.discardScopesUpTo(scopeIndex)
return js_ast.Property{}, false
}
Expand Down
8 changes: 4 additions & 4 deletions internal/js_parser/js_parser_lower_class.go
Original file line number Diff line number Diff line change
Expand Up @@ -1062,10 +1062,10 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, result visitClas
}
}

// If the field uses the TypeScript "declare" keyword, just omit it entirely.
// However, we must still keep any side-effects in the computed value and/or
// in the decorators.
if prop.Kind == js_ast.PropertyDeclare && prop.ValueOrNil.Data == nil {
// If the field uses the TypeScript "declare" or "abstract" keyword, just
// omit it entirely. However, we must still keep any side-effects in the
// computed value and/or in the decorators.
if prop.Kind == js_ast.PropertyDeclareOrAbstract && prop.ValueOrNil.Data == nil {
mustLowerField = true
shouldOmitFieldInitializer = true
}
Expand Down
8 changes: 6 additions & 2 deletions internal/js_parser/ts_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2051,9 +2051,11 @@ func TestTSExperimentalDecorator(t *testing.T) {
expectParseErrorExperimentalDecoratorTS(t, "@x export default abstract", "<stdin>: ERROR: Decorators are not valid here\n")
expectParseErrorExperimentalDecoratorTS(t, "@x export @y default class {}", "<stdin>: ERROR: Decorators are not valid here\n<stdin>: ERROR: Unexpected \"default\"\n")

// TypeScript experimental decorators are actually allowed on declared fields
// TypeScript experimental decorators are actually allowed on declared and abstract fields
expectPrintedExperimentalDecoratorTS(t, "class Foo { @(() => {}) declare foo: any; @(() => {}) bar: any }",
"class Foo {\n bar;\n}\n__decorateClass([\n () => {\n }\n], Foo.prototype, \"foo\", 2);\n__decorateClass([\n () => {\n }\n], Foo.prototype, \"bar\", 2);\n")
expectPrintedExperimentalDecoratorTS(t, "abstract class Foo { @(() => {}) abstract foo: any; @(() => {}) bar: any }",
"class Foo {\n bar;\n}\n__decorateClass([\n () => {\n }\n], Foo.prototype, \"foo\", 2);\n__decorateClass([\n () => {\n }\n], Foo.prototype, \"bar\", 2);\n")
}

func TestTSDecorators(t *testing.T) {
Expand Down Expand Up @@ -2130,9 +2132,11 @@ func TestTSDecorators(t *testing.T) {
expectParseErrorTS(t, "@x export default abstract", "<stdin>: ERROR: Decorators are not valid here\n")
expectParseErrorTS(t, "@x export @y default class {}", "<stdin>: ERROR: Decorators are not valid here\n<stdin>: ERROR: Unexpected \"default\"\n")

// JavaScript decorators are not allowed on declared fields
// JavaScript decorators are not allowed on declared or abstract fields
expectParseErrorTS(t, "class Foo { @(() => {}) declare foo: any; @(() => {}) bar: any }",
"<stdin>: ERROR: Decorators are not valid here\n")
expectParseErrorTS(t, "abstract class Foo { @(() => {}) abstract foo: any; @(() => {}) bar: any }",
"<stdin>: ERROR: Decorators are not valid here\n")
}

func TestTSTry(t *testing.T) {
Expand Down

0 comments on commit ae5cc17

Please sign in to comment.