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.

Sign up for GitHub

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

Jump to bottom

[babel 8] Remove support for the 2018-09 decorators proposal #12712

Merged
merged 10 commits into from Jun 26, 2022
Merged

[babel 8] Remove support for the 2018-09 decorators proposal #12712

merged 10 commits into from Jun 26, 2022

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Jan 29, 2021 •
edited by JLHwung

Q                       A
Fixed Issues? Closes #8750, closes #9454, closes #9220, closes #9602, closes #9999, closes #12591
Patch: Bug Fix?
Major: Breaking Change? Yes, behind the BABEL_8_BREAKING flag
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link babel/website#2765
Any Dependency Changes?
License MIT

This PR completely removes support for the second decorators proposal (the one with the big runtime code).

  1. We never fully supported the proposal (decorated private elements were left as a todo)
  2. That proposal version has been dropped on 2019-03-15, which is almost two years ago (tc39/proposal-decorators@a043d4b)
  3. After that version (which was the second) there have been other two versions

I'm not removing support for legacy decorators because many JS libraries depend on them.

Please review this PR with whitespace diff disabled, since different parts have been indented one level more (because of the if (BABEL_8_BREAKING) { statements).

@nicolo-ribaudo nicolo-ribaudo added this to the Babel 8.0 milestone Jan 29, 2021
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 29, 2021 •
edited

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 29, 2021 •
edited

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit faf85ff:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration
babel-repl-custom-plugin Issue #12591

JLHwung
JLHwung reviewed Feb 2, 2021
View reviewed changes
Copy link
Contributor

@JLHwung JLHwung left a comment •
edited by nicolo-ribaudo

Choose a reason for hiding this comment

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

We should also remove decoratorsBeforeExport from @babel/standalone.

EDIT by @nicolo-ribaudo: the new proposal version still supports this option

@nicolo-ribaudo nicolo-ribaudo force-pushed the breaking/remove-decorators branch 2 times, most recently from 940e6de to 70963b0 Compare June 19, 2021 01:05
@mbrowne
Copy link

mbrowne commented Jul 18, 2021

Just curious, when will work begin on an implementation of the latest decorators proposal? Or is it still not stable enough yet to be worth doing that at this point?

@nicolo-ribaudo
Copy link
Member Author

I'm in contact with the proposal authors, and it looks like it's becoming stable enough that we can implement it in the next months.

@wvq
Copy link

wvq commented Jan 2, 2022

For now, Is there any progress on stage 2 ?

@nicolo-ribaudo
Copy link
Member Author

We are working on it at #14004

@nicolo-ribaudo nicolo-ribaudo changed the title [babel 8] Remove support for the second decorators proposal [babel 8] Remove support for the 2018-09 decorators proposal Feb 13, 2022
@nicolo-ribaudo nicolo-ribaudo force-pushed the breaking/remove-decorators branch 2 times, most recently from 04d0c3a to 083c58a Compare February 15, 2022 22:58
@nicolo-ribaudo
Copy link
Member Author

The CI failure looks valid, I'll investigate.

@nicolo-ribaudo
Copy link
Member Author

CI failure fixed!

@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Mar 25, 2022
Open
34 tasks
@nicolo-ribaudo
Copy link
Member Author

Rebased, this is ready for review!

@nicolo-ribaudo nicolo-ribaudo force-pushed the breaking/remove-decorators branch 2 times, most recently from 68c71a5 to f849a44 Compare June 5, 2022 17:50
@nicolo-ribaudo
Copy link
Member Author

Rebased again

JLHwung
JLHwung reviewed Jun 24, 2022
View reviewed changes
packages/babel-plugin-syntax-decorators/src/index.ts Outdated
" requires a 'decoratorsBeforeExport' option, whose value must be a boolean.",
);
if (version === "2021-12") {
decoratorsBeforeExport = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it mean we can remove the decoratorsBeforeExport generator option in Babel 8?

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR still keeps the decoratorsBeforeExport option but makes it default to false, I think I can just remove it given the proposal is now stage 3.

@nicolo-ribaudo
Copy link
Member Author

@JLHwung I removed decoratorsBeforeExport from the plugin and from generator. I didn't remove it in the parser yet because I first want to make it optional in Babel 7 (so that I don't have to duplicate all the tests between Babel 7 and Babel 8), but it needs to wait for the next minor.

@nicolo-ribaudo nicolo-ribaudo merged commit a7c438c into babel:main Jun 26, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the breaking/remove-decorators branch June 26, 2022 09:25
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Jun 26, 2022
Merged
@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 Sep 26, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2022
@JLHwung JLHwung added PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release and removed babel 8 labels Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JLHwung JLHwung JLHwung approved these changes

@liuxingbaoyu liuxingbaoyu liuxingbaoyu approved these changes

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release Spec: Decorators
Projects
None yet
Milestone
v8.0.0
6 participants
@nicolo-ribaudo @babel-bot @mbrowne @wvq @JLHwung @liuxingbaoyu