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

Multiple inheritances for plugins #5527

Closed
wants to merge 2 commits into from

Conversation

yavorsky
Copy link
Member

Q A
Patch: Bug Fix? n
Major: Breaking Change? n
Minor: New Feature? y
Deprecations? n
Spec Compliancy? n
Tests Added/Pass? y
Fixed Tickets n
License MIT
Doc PR n
Dependency Changes n

This PR allows use array for inherits field.
I've faced some problems when we need to specify multiple plugins, but could only one. For example, for babel-plugin-transform-es2015-modules-commonjs to support rest-spread syntax we need to inherit from syntax-object-rest-spread and seems like it's impossible unless if we already inherit from transform-strict-mode. (#5469).

For now, we can check inherits field and in the case of array just merge all the plugins with the current plugin like we do for the single item.

@mention-bot
Copy link

@yavorsky, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hzoo, @forivall and @loganfsmyth to be potential reviewers.

@yavorsky yavorsky changed the title Multiple inheritances Multiple inheritances for plugins Mar 22, 2017
@babel-bot
Copy link
Collaborator

Hey @yavorsky! It looks like one or more of your builds have failed. I've copied the relevant info below to save you some time.

@loganfsmyth
Copy link
Member

I'm confused, why would transform-es2015-modules-commonjs need to inherit rest-spread?

Personally I'm not in favor of this because I think inherits is a hack. I'd rather have Babel 7 allow plugins to have "child plugins" just like presets do.

@yavorsky
Copy link
Member Author

yavorsky commented Mar 22, 2017

@loganfsmyth

I'm confused, why would transform-es2015-modules-commonjs need to inherit rest-spread?

Just with transform-es2015-modules-commonjs in plugins Babel will throw an error for:
export const {a, ...b} = {};

So even if current environment supports spread syntax, we can't use it for commonjs transformation.

export const {a} = {} works ok.

I didn't know that inherits is a hack. For me, it's just the only solution that could help for another PR or future PRs. I could close and direct my eyes to your proposition for babel 7 with child plugins.

@loganfsmyth
Copy link
Member

I'd expect export const {a, ...b} = {}; to be treated the same as const {a, ...b} = {}; export {a, b};. That seems like a bug in the commonjs module, I wouldn't expect it to load an unrelated syntax plugin.

@yavorsky yavorsky closed this Mar 22, 2017
@yavorsky yavorsky deleted the multiple-inheritances branch March 22, 2017 22:22
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 6, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 6, 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants