Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial implementation of the decorators proposal #3754

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ jobs:
if: matrix.os == 'ubuntu-latest'
run: make lib-typecheck

- name: Decorator Tests
if: matrix.os == 'ubuntu-latest'
run: make decorator-tests

- name: WebAssembly API Tests (browser)
if: matrix.os == 'ubuntu-latest'
run: make test-wasm-browser
Expand Down
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

* Implement the JavaScript decorators proposal ([#104](https://github.com/evanw/esbuild/issues/104))

With this release, esbuild now contains an implementation of the upcoming [JavaScript decorators proposal](https://github.com/tc39/proposal-decorators). This is the same feature that shipped in [TypeScript 5.0](https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#decorators). You can read more about them in that blog post and in this other (now slightly outdated) extensive blog post here: https://2ality.com/2022/10/javascript-decorators.html. Here's a quick example:

```js
const log = (fn, context) => function() {
console.log(`before ${context.name}`)
const it = fn.apply(this, arguments)
console.log(`after ${context.name}`)
return it
}

class Foo {
@log static foo() {
console.log('in foo')
}
}

// Logs "before foo", "in foo", "after foo"
Foo.foo()
```

Note that this feature is different than the existing "TypeScript experimental decorators" feature that esbuild already implements. It uses similar syntax but behaves very differently, and the two are not compatible (although it's sometimes possible to write decorators that work with both). TypeScript experimental decorators will still be supported by esbuild going forward as they have been around for a long time, are very widely used, and let you do certain things that are not possible with JavaScript decorators (such as decorating function parameters). By default esbuild will parse and transform JavaScript decorators, but you can tell esbuild to parse and transform TypeScript experimental decorators instead by setting `"experimentalDecorators": true` in your `tsconfig.json` file.

Probably at least half of the work for this feature went into creating a test suite that exercises many of the proposal's edge cases: https://github.com/evanw/decorator-tests. It has given me a reasonable level of confidence that esbuild's initial implementation is acceptable. However, I don't have access to a significant sample of real code that uses JavaScript decorators. If you're currently using JavaScript decorators in a real code base, please try out esbuild's implementation and let me know if anything seems off.

**⚠️ WARNING ⚠️**

This proposal has been in the works for a very long time (work began around 10 years ago in 2014) and it is finally getting close to becoming part of the JavaScript language. However, it's still a work in progress and isn't a part of JavaScript yet, so keep in mind that any code that uses JavaScript decorators may need to be updated as the feature continues to evolve. The decorators proposal is pretty close to its final form but it can and likely will undergo some small behavioral adjustments before it ends up becoming a part of the standard. If/when that happens, I will update esbuild's implementation to match the specification. I will not be supporting old versions of the specification.

* Optimize the generated code for private methods

Previously when lowering private methods for old browsers, esbuild would generate one `WeakSet` for each private method. This mirrors similar logic for generating one `WeakSet` for each private field. Using a separate `WeakMap` for private fields is necessary as their assignment can be observable:
Expand Down
12 changes: 11 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test:
@$(MAKE) --no-print-directory -j6 test-common

# These tests are for development
test-common: test-go vet-go no-filepath verify-source-map end-to-end-tests js-api-tests plugin-tests register-test node-unref-tests
test-common: test-go vet-go no-filepath verify-source-map end-to-end-tests js-api-tests plugin-tests register-test node-unref-tests decorator-tests

# These tests are for release (the extra tests are not included in "test" because they are pretty slow)
test-all:
Expand Down Expand Up @@ -85,6 +85,16 @@ end-to-end-tests: version-go
node scripts/esbuild.js npm/esbuild/package.json --version
node scripts/end-to-end-tests.js

# Note: The TypeScript source code for these tests was copied from the repo
# https://github.com/evanw/decorator-tests, which is the official location of
# the source code for these tests. Any changes to these tests should be made
# there first and then copied here afterward.
decorator-tests: esbuild
./esbuild scripts/decorator-tests.ts --target=es2022 --outfile=scripts/decorator-tests.js
node scripts/decorator-tests.js
node scripts/decorator-tests.js | grep -q 'All checks passed'
git diff --exit-code scripts/decorator-tests.js

js-api-tests: version-go
node scripts/esbuild.js npm/esbuild/package.json --version
node scripts/js-api-tests.js
Expand Down
4 changes: 0 additions & 4 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6641,15 +6641,11 @@ func (p *parser) parseDecorators(decoratorScope *js_ast.Scope, classKeyword logg
p.log.AddErrorWithNotes(&p.tracker, p.lexer.Range(), "Parameter decorators only work when experimental decorators are enabled", []logger.MsgData{{
Text: "You can enable experimental decorators by adding \"experimentalDecorators\": true to your \"tsconfig.json\" file.",
}})
} else {
p.markSyntaxFeature(compat.Decorators, p.lexer.Range())
}
}
} else {
if (context & decoratorInFnArgs) != 0 {
p.log.AddError(&p.tracker, p.lexer.Range(), "Parameter decorators are not allowed in JavaScript")
} else {
p.markSyntaxFeature(compat.Decorators, p.lexer.Range())
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions internal/js_parser/js_parser_lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ func (p *parser) markSyntaxFeature(feature compat.JSFeature, r logger.Range) (di
case compat.NestedRestBinding:
name = "non-identifier array rest patterns"

case compat.Decorators:
name = "JavaScript decorators"

case compat.ImportAttributes:
p.log.AddError(&p.tracker, r, fmt.Sprintf(
"Using an arbitrary value as the second argument to \"import()\" is not possible in %s", where))
Expand Down
Loading
Loading