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

Allow to specify options for separate plugins in `preset-env` #6978

Open
th0r opened this Issue Dec 5, 2017 · 10 comments

Comments

Projects
None yet
6 participants
@th0r

th0r commented Dec 5, 2017

It's a feature request.

Context

Sometimes you need to specify options for some plugins inside preset-env e.g. when you want to enable loose mode only for some of them.
The only way to do it right now is to include those plugins separately under the plugins section of babel config, but in this case they will be used even for the targets that don't actually need them.

Here is the concrete example:

I want to use preset-env in loose mode but I can't enable loose mode for transform-spread because in this case it doesn't transform code like this [...someMap.value()] properly.

So the only solution right now is to use config like that:

{
  "presets": [
    ["@babel/preset-env", {"loose": true}]
  ],

  "plugins": [
    ["@babel/plugin-transform-spread", {"loose": false}]
  ]
}

But in this case spread operator will be transformed even if target browser supports this syntax natively.

Possible Solution

Introduce an option for preset-env that will contain options for separate plugins:

{
  "presets": [
    ["@babel/preset-env", {
      "loose": true,
      "pluginOptions": {
        "@babel/plugin-transform-spread": {"loose": false}
      }
    }]
  ]
}

They will override any options provided by preset-env by itself e.g. in the case above loose: false will override loose: true for transform-spread plugin.

Your Environment

babel: 7.0.0-beta.32
preset-env: 7.0.0-beta.32

@babel-bot

This comment has been minimized.

Show comment
Hide comment
@babel-bot

babel-bot Dec 5, 2017

Collaborator

Hey @th0r! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

Collaborator

babel-bot commented Dec 5, 2017

Hey @th0r! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@th0r

This comment has been minimized.

Show comment
Hide comment
@th0r

th0r Dec 6, 2017

I can try to implement this feature, I just need confirmation that this proposal makes sense.

th0r commented Dec 6, 2017

I can try to implement this feature, I just need confirmation that this proposal makes sense.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 6, 2017

Member

I've generally been against this in the past, but I'm also happy to continue the discussion. I'm kind of split on it.

Requests for this, to me, usually imply that our current options have flaws and/or aren't granular enough. That is definitely the case for loose because no one knows what they are actually opting into because it has no consistent goal.

To me, the preferred approach to that problem would be to make better options with clear names.

That said, you could certainly also make an argument that it is better to let users to whatever, and make better options to do what users want later. I worry that we'll end up hearing less from users when they want features because they will just do workarounds using this instead.

Member

loganfsmyth commented Dec 6, 2017

I've generally been against this in the past, but I'm also happy to continue the discussion. I'm kind of split on it.

Requests for this, to me, usually imply that our current options have flaws and/or aren't granular enough. That is definitely the case for loose because no one knows what they are actually opting into because it has no consistent goal.

To me, the preferred approach to that problem would be to make better options with clear names.

That said, you could certainly also make an argument that it is better to let users to whatever, and make better options to do what users want later. I worry that we'll end up hearing less from users when they want features because they will just do workarounds using this instead.

@th0r

This comment has been minimized.

Show comment
Hide comment
@th0r

th0r Dec 6, 2017

@loganfsmyth Well, if I understood you correctly, your main target is to make preset-env as simple and user-friendly as possible and ideally loose option should be smart enough to switch off loose mode for plugins automatically if codebase contains code that won't be transformed properly, right?

So from your point of view pluginOptions is a workaround and not the true solution?

th0r commented Dec 6, 2017

@loganfsmyth Well, if I understood you correctly, your main target is to make preset-env as simple and user-friendly as possible and ideally loose option should be smart enough to switch off loose mode for plugins automatically if codebase contains code that won't be transformed properly, right?

So from your point of view pluginOptions is a workaround and not the true solution?

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 6, 2017

Member

ideally loose option should be smart enough to switch off loose mode for plugins automatically if codebase contains code that won't be transformed properly

The issue for me is that there isn't a definition of "properly transformed". There's no way to turn if off loose if things won't transform properly because whatever loose happens to do by definition matches what it seems as "proper". The definition of loose, as far as it exists, is essentially "do some stuff that isn't spec compliant". The fact that you want to use it on some plugins and not others means that we should have better options with names that align with a particular compilation goal. I personally wouldn't recommend loose mode to anyone ever because of how poorly defined it is, and I see the fact that people use it as indication that we aren't properly aligning our options with user expectations.

So from your point of view pluginOptions is a workaround and not the true solution?

I think for the case of loose it is almost certainly a workaround. I'd much rather we add more options that actually had well-defined behavior.

Member

loganfsmyth commented Dec 6, 2017

ideally loose option should be smart enough to switch off loose mode for plugins automatically if codebase contains code that won't be transformed properly

The issue for me is that there isn't a definition of "properly transformed". There's no way to turn if off loose if things won't transform properly because whatever loose happens to do by definition matches what it seems as "proper". The definition of loose, as far as it exists, is essentially "do some stuff that isn't spec compliant". The fact that you want to use it on some plugins and not others means that we should have better options with names that align with a particular compilation goal. I personally wouldn't recommend loose mode to anyone ever because of how poorly defined it is, and I see the fact that people use it as indication that we aren't properly aligning our options with user expectations.

So from your point of view pluginOptions is a workaround and not the true solution?

I think for the case of loose it is almost certainly a workaround. I'd much rather we add more options that actually had well-defined behavior.

@th0r

This comment has been minimized.

Show comment
Hide comment
@th0r

th0r Dec 6, 2017

I personally wouldn't recommend loose mode to anyone ever because of how poorly defined it is, and I see the fact that people use it as indication that we aren't properly aligning our options with user expectations.

I use loose mode because it results in more clean, readable and performant code. I have carefully read all the warnings about it and I'm perfectly fine with them.

I'd much rather we add more options that actually had well-defined behavior.

But it will be just renaming of plugin options and their list will only grow when preset-env will support next ES versions.

th0r commented Dec 6, 2017

I personally wouldn't recommend loose mode to anyone ever because of how poorly defined it is, and I see the fact that people use it as indication that we aren't properly aligning our options with user expectations.

I use loose mode because it results in more clean, readable and performant code. I have carefully read all the warnings about it and I'm perfectly fine with them.

I'd much rather we add more options that actually had well-defined behavior.

But it will be just renaming of plugin options and their list will only grow when preset-env will support next ES versions.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Dec 6, 2017

Member

I use loose mode because it results in more clean, readable and performant code.

I think I could be clearer with examples. I'm all for options that simplify the code, but I'm assuming in your case you actually want to use iterables, right? So I'd be way more motivated to have an option like assumeArray to make Babel's output assume all places that iterables come up are just arrays. That's what loose does, and what you're opting out of for spread right? Similarly a lot of people want to avoid Object.defineProperty so we could have an option to avoid that, which simplifies readability of classes and things, so we could have a noDefineProperty option.

Member

loganfsmyth commented Dec 6, 2017

I use loose mode because it results in more clean, readable and performant code.

I think I could be clearer with examples. I'm all for options that simplify the code, but I'm assuming in your case you actually want to use iterables, right? So I'd be way more motivated to have an option like assumeArray to make Babel's output assume all places that iterables come up are just arrays. That's what loose does, and what you're opting out of for spread right? Similarly a lot of people want to avoid Object.defineProperty so we could have an option to avoid that, which simplifies readability of classes and things, so we could have a noDefineProperty option.

@th0r

This comment has been minimized.

Show comment
Hide comment
@th0r

th0r Dec 7, 2017

That's exactly what I meant by "it will be just renaming of plugin options and their list will only grow when preset-env will support next ES versions".

Do you want preset-env to be some kind of black box that doesn't reveal its internals and plugins that are used under the hood? I mean what you propose is just renaming e.g. loose option for transform-spread into assumeArray.
But preset-env is already not a black box because of options like include and exclude. Actually, from your point of view they are also "workarounds". Were you agains them as well?

Also I have the following concerns:

  1. Are you sure that we'll be able to find nice names for all the existing and future plugin options? Do you really want to do this work?
  2. Let's assume that there are people, that currently use manually created list of es2015 plugins with options and who want to move to preset-env. Renamed options will only introduce extra difficulties and confusion.

th0r commented Dec 7, 2017

That's exactly what I meant by "it will be just renaming of plugin options and their list will only grow when preset-env will support next ES versions".

Do you want preset-env to be some kind of black box that doesn't reveal its internals and plugins that are used under the hood? I mean what you propose is just renaming e.g. loose option for transform-spread into assumeArray.
But preset-env is already not a black box because of options like include and exclude. Actually, from your point of view they are also "workarounds". Were you agains them as well?

Also I have the following concerns:

  1. Are you sure that we'll be able to find nice names for all the existing and future plugin options? Do you really want to do this work?
  2. Let's assume that there are people, that currently use manually created list of es2015 plugins with options and who want to move to preset-env. Renamed options will only introduce extra difficulties and confusion.
@th0r

This comment has been minimized.

Show comment
Hide comment
@th0r

th0r Dec 14, 2017

@loganfsmyth if I still couldn't convince you let's then try to find better names for options?

th0r commented Dec 14, 2017

@loganfsmyth if I still couldn't convince you let's then try to find better names for options?

@ylemkimon

This comment has been minimized.

Show comment
Hide comment
@ylemkimon

ylemkimon Jan 12, 2018

Currently only loose option can be passed to the preset configuration, which is passed globally to the plugins and their behavior is different per plugin. I think we should be at least able to pass other options to plugins.

ylemkimon commented Jan 12, 2018

Currently only loose option can be passed to the preset configuration, which is passed globally to the plugins and their behavior is different per plugin. I think we should be at least able to pass other options to plugins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment