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

Make @babel/plugin-class-features a normal helper package #9083

Merged

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Nov 25, 2018

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

This is a better alternative to #9050.


This PR removes the @babel/plugin-class-features package, which becames @babel/helper-create-class-features-plugin and can only be used inside other plugins.

The problem with the @babel/plugin-class-features plugin is that, in order to allow using @babel/plugin-proposal-class-properties, etc., it behaves differently when declared multiple times:

{
  "plugins": [
    ["@babel/plugin-class-features", { "fields": true }],
    ["@babel/plugin-class-features", { "decorators": true }]
  ]
}

For any other plugin, the last specified options override the others, but in this case they are merged.

In #9050 I removed direct options to that plugin, but it doesn't make sense to allow using it as plugins: ["@babel/plugin-class-features"] since it does nothing.

@nicolo-ribaudo nicolo-ribaudo added PR: Internal 🏠 A type of pull request used for our changelog categories Spec: Class Fields labels Nov 25, 2018
@nicolo-ribaudo nicolo-ribaudo added this to To do in Class features via automation Nov 25, 2018
@nicolo-ribaudo nicolo-ribaudo moved this from To do to In progress in Class features Nov 25, 2018
@xtuc xtuc mentioned this pull request Nov 25, 2018
4 tasks
@babel-bot
Copy link
Collaborator

babel-bot commented Nov 25, 2018

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

@@ -1,5 +1,5 @@
{
"name": "@babel/plugin-class-features",
"name": "@babel/helper-class-features-plugin",
Copy link
Member

Choose a reason for hiding this comment

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

maybe @babel/helper-create-class-feature-plugin or @babel/helper-class-feature-factory?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the first name 👍

Copy link
Member

Choose a reason for hiding this comment

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

@babel/helper-create-class-feature-plugin 👍

Copy link
Member

@hzoo hzoo left a comment

Choose a reason for hiding this comment

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

awesome, it's a good idea. It seems weird to have another way to turn on a plugin (otherwise I would move all experimental features into the same plugin). Each plugin is separately turned on in a consistent way this way.

return {
name: "class-features",
name: "class-features/" + name,
Copy link
Member

Choose a reason for hiding this comment

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

I think I'd rather keep the names consistent with the other plugins. Maybe we can delete this line and have the plugin do:

return {
  name: "...",
  ...createClassFeaturePlugin({ /* ... */ })
};

?

nicolo-ribaudo and others added 5 commits November 29, 2018 15:29
This effectively disallows using it directly.
Co-Authored-By: nicolo-ribaudo <nicolo.ribaudo@gmail.com>
@nicolo-ribaudo nicolo-ribaudo merged commit 4e28459 into babel:master Nov 29, 2018
Class features automation moved this from In progress to Done Nov 29, 2018
@nicolo-ribaudo nicolo-ribaudo deleted the remove-plugin-class-features branch November 29, 2018 15:42
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019
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: Internal 🏠 A type of pull request used for our changelog categories
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants