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

Add 2022-03 decorators version (stage 3) #14836

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Aug 8, 2022

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR adds a new version of the decorators plugin, representing the stage 3 proposal that reached consensus in the March 2022 TC39 meeting: #14836 (comment)

Also, the decoratorsBeforeExport option is disallowed with the new version and decorators are always after export (this aligns with the Babel 8 behavior).

ℹ️ The first two commits only copy the 2021-12 files to 2022-03. I suggest hiding it when reviewing (https://github.com/babel/babel/pull/14836/files/915df0ba082dd55dfe5ddc0abf0967ac2ecb571c..8dd440618dbd1fd75fe467b272ccb1859f8c7865), to see the diff between the 2022-03 code/tests and the 2021-12 code/tests.

cc @pzuraq @jkrems

@nicolo-ribaudo nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Decorators labels Aug 8, 2022
@nicolo-ribaudo nicolo-ribaudo added this to the 7.19.0 milestone Aug 8, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Aug 8, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52770/

Copy link
Contributor

@jkrems jkrems left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only skimmed and left a quick question - thanks for working on this!

expect(bContext.static).toBe(false);
expect(bContext.private).toBe(true);
expect(typeof bContext.addInitializer).toBe('function');
expect(typeof bContext.setMetadata).toBe('function');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the expected scope for the babel implementation are but setMetadata/getMetadata have been removed in the March update: tc39/proposal-decorators@5c1cc15

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes thanks I should remove them! Are there any other runtime changes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let @pzuraq take that. I've been only following the proposal progress on the sidelines. AFAIK the is* renaming and metadata split were the two notable changes. But it looks like there also might have been a unification of access..? tc39/proposal-decorators@13e07b3

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, unification of access was one other change we made. I'll comment with a list of all changes

@pzuraq
Copy link
Contributor

pzuraq commented Aug 8, 2022

Thanks for pushing this forward @nicolo-ribaudo! I've been meaning to and just haven't had time.

Here are all the major changes we should make sure to get in this round:

  1. initialize on the return value for accessors has been renamed to init (this may have already been addressed)
  2. isPrivate and isStatic renamed to private and static
  3. Metadata has been removed
  4. access is available for all public and private class elements
  5. @(expr)() is no longer allowed, must wrap the whole expression in parens

I believe that's everything, reviewing the repo issues and PRs. I haven't had a chance to dig back into the actual spec changes, I'll try to find some time this week to review those and make sure nothing slipped between the cracks.

@nicolo-ribaudo nicolo-ribaudo changed the title Add 2022-03 decorators version (runtime semantics) Add 2022-03 decorators version (stage 3) Aug 8, 2022
@nicolo-ribaudo
Copy link
Member Author

Thanks @pzuraq! I updated the PR.

@JLHwung
Copy link
Contributor

JLHwung commented Aug 8, 2022

It seems to me #14666 should be part of this PR, but that one is pending tc39/ecma262#2417 (comment). I think we intend to allow @foo.#bar as decorator but not @#bar. @pzuraq Can you confirm?

@@ -0,0 +1,619 @@
/* @minVersion 7.17.8 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be 7.19.xx?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be updated when publishing, some tests fail when helpers specify a future version.

@nicolo-ribaudo nicolo-ribaudo force-pushed the decorators-stage-3-updates branch 2 times, most recently from ca5415c to 65de4e7 Compare August 13, 2022 10:15
@nicolo-ribaudo
Copy link
Member Author

I don't know why the e2e test is failing 😕

@nicolo-ribaudo
Copy link
Member Author

I can reproduce the failure locally downloading the CI artifact

@nicolo-ribaudo nicolo-ribaudo changed the base branch from main to feat-7.19.0/decorators August 17, 2022 14:40
@nicolo-ribaudo
Copy link
Member Author

@JLHwung
Copy link
Contributor

JLHwung commented Aug 17, 2022

Since #14353 is merged and shipped in decorator version 2021-12 and I assume that should have been a 2022-03 thing, we may want to clarify the differences between 2021-12 and 2022-03 in docs:

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 17, 2022

I would word it as:

  • decorators valid according to the proposal as it was presented on 2021-12 can be run using version: 2021-12
  • decorators valid according to the proposal as it was presented on 2022-03 can be run using version: 2022-03, and it will not work with invalid decorators

because the spec updates we merged to 2021-12 are all backward compatible anyway.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Aug 22, 2022

I prefer to always avoid it, it's simpler and generates future-proof code!

EDIT: It already works like that:


@JLHwung
Copy link
Contributor

JLHwung commented Aug 22, 2022

Not if one enabled createParenthesizedExpressions:

@nicolo-ribaudo nicolo-ribaudo merged commit a05b13b into babel:feat-7.19.0/decorators Aug 30, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the decorators-stage-3-updates branch August 30, 2022 09:42
nicolo-ribaudo added a commit that referenced this pull request Sep 2, 2022
* Copy `applyDecs`->`applyDecs2203` helper, and `2021-12`->`2022-03` tests

* Avoid conflicting fn names in helpers

* Add `2022-03` decorators version

* Rename `isPrivate`/`isStatic`->`private`/`static`

* Disallow `@(...)()` in 2022-03 decorators

This commits add a new `allowCallParenthesized` option to the decorators parser plugin:
when set to `false`, `@(...)()`-style decorators are allowed to match the stage 3
proposal presented in March 2022.

It will default to `false` in Babel 8 (we might just remove the option)

* Disallow `decoratorsBeforeExport` option with 2022-03 decorators

This aligns with the Babel 8 behavior

* Remove `.initializer` fallback for accessor properties

* Remove `.initializer` fallback for accessor properties

* Expose `.access` for public class elements

* Remove `getMetadata`/`setMetadata`

* Make the parser error recoverable

* Print necessary parentheses in generator
nicolo-ribaudo added a commit that referenced this pull request Sep 2, 2022
* Copy `applyDecs`->`applyDecs2203` helper, and `2021-12`->`2022-03` tests

* Avoid conflicting fn names in helpers

* Add `2022-03` decorators version

* Rename `isPrivate`/`isStatic`->`private`/`static`

* Disallow `@(...)()` in 2022-03 decorators

This commits add a new `allowCallParenthesized` option to the decorators parser plugin:
when set to `false`, `@(...)()`-style decorators are allowed to match the stage 3
proposal presented in March 2022.

It will default to `false` in Babel 8 (we might just remove the option)

* Disallow `decoratorsBeforeExport` option with 2022-03 decorators

This aligns with the Babel 8 behavior

* Remove `.initializer` fallback for accessor properties

* Remove `.initializer` fallback for accessor properties

* Expose `.access` for public class elements

* Remove `getMetadata`/`setMetadata`

* Make the parser error recoverable

* Print necessary parentheses in generator
nicolo-ribaudo added a commit that referenced this pull request Sep 5, 2022
* Copy `applyDecs`->`applyDecs2203` helper, and `2021-12`->`2022-03` tests

* Avoid conflicting fn names in helpers

* Add `2022-03` decorators version

* Rename `isPrivate`/`isStatic`->`private`/`static`

* Disallow `@(...)()` in 2022-03 decorators

This commits add a new `allowCallParenthesized` option to the decorators parser plugin:
when set to `false`, `@(...)()`-style decorators are allowed to match the stage 3
proposal presented in March 2022.

It will default to `false` in Babel 8 (we might just remove the option)

* Disallow `decoratorsBeforeExport` option with 2022-03 decorators

This aligns with the Babel 8 behavior

* Remove `.initializer` fallback for accessor properties

* Remove `.initializer` fallback for accessor properties

* Expose `.access` for public class elements

* Remove `getMetadata`/`setMetadata`

* Make the parser error recoverable

* Print necessary parentheses in generator
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Nov 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Needs Docs PR: New Feature 🚀 A type of pull request used for our changelog categories Spec: Decorators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants