Skip to content

Commit

Permalink
fix #734: ts decorators on class ctor args
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Feb 3, 2021
1 parent e4a423f commit 6de420b
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 3 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

## Unreleased

* Fix TypeScript parameter decorators on class constructors ([#734](https://github.com/evanw/esbuild/issues/734))

This release fixes a TypeScript translation bug where parameter decorators on class constructors were translated incorrectly. Affected code looks like this:

```js
class Example {
constructor(@decorator param: any) {}
}
```

This bug has been fixed. In addition, decorators are no longer allowed on class constructors themselves because they are not allowed in TypeScript.

* Resolve `browser` entries in `package.json` with no file extension ([#740](https://github.com/evanw/esbuild/issues/740))

This fix changes how esbuild interprets the `browser` field in `package.json`. It will now remap imports without a file extension to `browser` map entries without a file extension, which improves compatibility with Webpack. Specifically, a `package.json` file with `"browser": {"./file": "./something.js"}` will now match an import of `./file`. Previously the `package.json` file had to contain something like `"browser": {"./file.js": "./something.js"}` instead. Note that for compatibility with the rest of the ecosystem, a remapping of `./file` will counter-intuitively _not_ match an import of `./file.js` even though it works fine in the other direction.
Expand Down
1 change: 1 addition & 0 deletions internal/bundler/bundler_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@ func TestTypeScriptDecorators(t *testing.T) {
export default class Foo {
@x @y mUndef
@x @y mDef = 1
constructor(@x0 @y0 arg0, @x1 @y1 arg1) {}
@x @y method(@x0 @y0 arg0, @x1 @y1 arg1) { return new Foo }
@x @y static sUndef
@x @y static sDef = new Foo
Expand Down
8 changes: 6 additions & 2 deletions internal/bundler/snapshots/snapshots_ts.txt
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ TestTypeScriptDecorators
---------- /out.js ----------
// all.ts
var Foo = class {
constructor() {
constructor(arg0, arg1) {
this.mDef = 1;
}
method(arg0, arg1) {
Expand Down Expand Up @@ -356,7 +356,11 @@ __decorate([
], Foo, "sMethod", 1);
Foo = __decorate([
x.y(),
new y.x()
new y.x(),
__param(0, x0),
__param(0, y0),
__param(1, x1),
__param(1, y1)
], Foo);
var all_default = Foo;

Expand Down
8 changes: 8 additions & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -4501,13 +4501,21 @@ func (p *parser) parseClass(name *js_ast.LocRef, classOpts parseClassOpts) js_as
}

// Parse decorators for this property
firstDecoratorLoc := p.lexer.Loc()
if opts.allowTSDecorators {
opts.tsDecorators = p.parseTypeScriptDecorators()
}

// This property may turn out to be a type in TypeScript, which should be ignored
if property, ok := p.parseProperty(js_ast.PropertyNormal, opts, nil); ok {
properties = append(properties, property)

// Forbid decorators on class constructors
if len(opts.tsDecorators) > 0 {
if key, ok := property.Key.Data.(*js_ast.EString); ok && js_lexer.UTF16EqualsString(key.Value, "constructor") {
p.log.AddError(&p.source, firstDecoratorLoc, "TypeScript does not allow decorators on class constructors")
}
}
}
}

Expand Down
10 changes: 9 additions & 1 deletion internal/js_parser/js_parser_lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -1585,10 +1585,18 @@ func (p *parser) lowerClass(stmt js_ast.Stmt, expr js_ast.Expr, shadowRef js_ast
// Merge parameter decorators with method decorators
if p.options.ts.Parse && prop.IsMethod {
if fn, ok := prop.Value.Data.(*js_ast.EFunction); ok {
isConstructor := false
if key, ok := prop.Key.Data.(*js_ast.EString); ok {
isConstructor = js_lexer.UTF16EqualsString(key.Value, "constructor")
}
for i, arg := range fn.Fn.Args {
for _, decorator := range arg.TSDecorators {
// Generate a call to "__param()" for this parameter decorator
prop.TSDecorators = append(prop.TSDecorators,
var decorators *[]js_ast.Expr = &prop.TSDecorators
if isConstructor {
decorators = &class.TSDecorators
}
*decorators = append(*decorators,
p.callRuntime(decorator.Loc, "__param", []js_ast.Expr{
{Loc: decorator.Loc, Data: &js_ast.ENumber{Value: float64(i)}},
decorator,
Expand Down
4 changes: 4 additions & 0 deletions internal/js_parser/ts_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,10 @@ func TestTSDecorator(t *testing.T) {
expectParseErrorTS(t, "class Foo { @dec static *#foo() {} }", "<stdin>: error: Expected identifier but found \"#foo\"\n")
expectParseErrorTS(t, "class Foo { @dec static async #foo() {} }", "<stdin>: error: Expected identifier but found \"#foo\"\n")
expectParseErrorTS(t, "class Foo { @dec static async* #foo() {} }", "<stdin>: error: Expected identifier but found \"#foo\"\n")

// Decorators aren't allowed on class constructors
expectParseErrorTS(t, "class Foo { @dec constructor() {} }", "<stdin>: error: TypeScript does not allow decorators on class constructors\n")
expectParseErrorTS(t, "class Foo { @dec public constructor() {} }", "<stdin>: error: TypeScript does not allow decorators on class constructors\n")
}

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

0 comments on commit 6de420b

Please sign in to comment.