Skip to content

Commit

Permalink
ts: forbid regular decorators on declare fields
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Nov 23, 2023
1 parent 69c9e7f commit e755189
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
2 changes: 0 additions & 2 deletions internal/bundler_tests/bundler_ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -990,14 +990,12 @@ func TestTSExperimentalDecoratorsNoConfig(t *testing.T) {
@x @y mUndef: any
@x @y mDef = 1
@x @y method() { return new Foo }
@x @y declare mDecl: any
@x @y accessor aUndef: any
@x @y accessor aDef = 1
@x @y static sUndef: any
@x @y static sDef = new Foo
@x @y static sMethod() { return new Foo }
@x @y static declare sDecl: any
@x @y static accessor asUndef: any
@x @y static accessor asDef = 1
Expand Down
18 changes: 16 additions & 2 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2181,10 +2181,11 @@ func (p *parser) parseProperty(startLoc logger.Loc, kind js_ast.PropertyKind, op
scopeIndex := len(p.scopesInOrder)

if prop, ok := p.parseProperty(startLoc, kind, opts, nil); ok &&
prop.Kind == js_ast.PropertyNormal && prop.ValueOrNil.Data == nil && len(opts.decorators) > 0 {
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 "declare" keyword,
// only keep the declaration to preserve its side-effects when
// there are TypeScript decorators present:
// there are TypeScript experimental decorators present:
//
// class Foo {
// // Remove this
Expand All @@ -2194,6 +2195,15 @@ func (p *parser) parseProperty(startLoc logger.Loc, kind js_ast.PropertyKind, op
// @decorator(console.log('side effect 2')) declare bar
// }
//
// This behavior is surprisingly somehow valid with TypeScript
// experimental decorators, which was possibly by accident.
// TypeScript does not allow this with JavaScript decorators.
//
// References:
//
// https://github.com/evanw/esbuild/issues/1675
// https://github.com/microsoft/TypeScript/issues/46345
//
prop.Kind = js_ast.PropertyDeclare
return prop, true
}
Expand Down Expand Up @@ -6241,6 +6251,7 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp

// Parse decorators for this property
firstDecoratorLoc := p.lexer.Loc()
scopeIndex := len(p.scopesInOrder)
opts.decorators = p.parseDecorators(p.currentScope, classKeyword, opts.decoratorContext)

// This property may turn out to be a type in TypeScript, which should be ignored
Expand All @@ -6261,6 +6272,9 @@ func (p *parser) parseClass(classKeyword logger.Range, name *ast.LocRef, classOp
hasConstructor = true
}
}
} else if !classOpts.isTypeScriptDeclare && len(opts.decorators) > 0 {
p.log.AddError(&p.tracker, logger.Range{Loc: firstDecoratorLoc, Len: 1}, "Decorators are not valid here")
p.discardScopesUpTo(scopeIndex)
}
}

Expand Down
8 changes: 8 additions & 0 deletions internal/js_parser/ts_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2029,6 +2029,10 @@ func TestTSExperimentalDecorator(t *testing.T) {
expectParseErrorExperimentalDecoratorTS(t, "@x export @y class Foo {}", "<stdin>: ERROR: Decorators are not valid here\n")
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
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")
}

func TestTSDecorators(t *testing.T) {
Expand Down Expand Up @@ -2104,6 +2108,10 @@ func TestTSDecorators(t *testing.T) {
expectParseErrorTS(t, "@x export @y class Foo {}", "<stdin>: ERROR: Decorators are not valid here\n")
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
expectParseErrorTS(t, "class Foo { @(() => {}) declare foo: any; @(() => {}) bar: any }",
"<stdin>: ERROR: Decorators are not valid here\n")
}

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

0 comments on commit e755189

Please sign in to comment.