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

Move "targets"-related options to @babel/core #2

Merged
merged 10 commits into from
Feb 12, 2021

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Apr 25, 2020

View Rendered Text

This RFC proposes moving the Browserslist integration from @babel/preset-env to @babel/core. This means deprecating all the Browserslists-related option from the preset and introducing new top-level configuration options.

PR: babel/babel#12189


This is the first time we test the new "standardized" RFC process, let's hope that it works 🤞

I'm pinging a few people who might be interested in this:

Please share this RFC with anyone who can contribute to the discussion!

@ai
Copy link

ai commented Apr 26, 2020

Should .browserslistrc files be considered as project-wide or file-relative configs?

We created these files as a file-relative config to have a special dir legacy/ with another target browser.

In reality, it is a rare case. At least, minifier should use a project-wide Browserslist in the project root.

Is the "Merge the new options in a single object" better than having three new top-level options?

I think, that most of the projects should avoid using browserslistConfigFile config. The core idea of the Browserslist is to have the same target browsers in all tools. With a config in a non-standard place, it will be hard to achieve.

For the same reason, I think target should be avoided (and used only for rare tricky cases).

In the end, I think that only browserslistEnv could be a popular option. So adding one global browserslist: { targets, envName, configFile } will not be so useful.

@devongovett
Copy link

devongovett commented Apr 26, 2020

This is great. At the moment, Parcel has its own wrapper around preset-env: @parcel/babel-preset-env. This is so we can pass our targets through during multi-target builds rather than hard coding the targets in the babel config. If this RFC is implemented, we could get rid of this wrapper and Parcel could simply pass the targets to babel core instead.

There is another case where we need to create a wrapper for the same reasons: @parcel/babel-transform-runtime. This is so we can switch between ESM and CommonJS helpers based on the output format of the bundle in multi-target builds. It might be nice to move the module format to a top-level babel option as well, which would be used by preset-env, transform-runtime, etc. Thoughts?

@hzoo
Copy link
Member

hzoo commented Apr 27, 2020

FYI related to this proposal is simply the idea we just move preset-env into @babel/core as well - didn't do an RFC yet but closely tied to this #3. Maybe better to just do both at once, wondering how we can do this in a minor?

@ianschmitz
Copy link

FYI related to this proposal is simply the idea we just move preset-env into @babel/core as well - didn't do an RFC yet but closely tied to this #3. Maybe better to just do both at once, wondering how we can do this in a minor?

Potentially we could move the @babel/preset-env logic into @babel/core. We could still continue to support @babel/preset-env by having the inclusion of the preset enable the functionality within core? In next major @babel/preset-env could melt away.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Oct 16, 2020

I have prepared a PR for this RFC (babel/babel#12189), and made some small updated here with what I found while implementing it.


Sorry if I have been super slow (= 6 months? 😅) to reply to the comments.

There is another case where we need to create a wrapper for the same reasons: @parcel/babel-transform-runtime. This is so we can switch between ESM and CommonJS helpers based on the output format of the bundle in multi-target builds. It might be nice to move the module format to a top-level babel option as well, which would be used by preset-env, transform-runtime, etc.

This will likely be fixed by using exports in @babel/runtime's package.json:

"exports": {
  "./helpers/foo": {
    "import": "./helpers/esm/foo.js",
    "default": "./helpers/foo.js"
  }
}

Potentially we could move the @babel/preset-env logic into @babel/core.

That is one of my end goals, but I want to that slowly to use this opportunity to also improve the general API. This RFC and #5 both move in that direction.


## New Babel options

### `targets`
Copy link
Collaborator

@JLHwung JLHwung Oct 19, 2020

Choose a reason for hiding this comment

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

How should "targets" be merged? It seems that [].concat (like how we did for presets) suffices but we should document about that as it may be out of expectation:

If babel-preset-esmodule specifies targets: "chrome 52, safari 14.1, node 13.2" and babel-preset-node specifies targets: node >= 8, the resolved targets will be ["chrome 52, safari 14.1, node 13.2", "node >= 8"], which is the union of these two targets.

But when targets is an object, our config merging (by default) will make intersect these two targets. We should specialize the merging of targets.

{
  chrome: 52,
  safari: "14.1",
  node: "13.2"
}  + { node: "14.0" }
== {
  chrome: 52,
  safari: "14.1",
  node: "14.0"
} 

Choose a reason for hiding this comment

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

Another edge case for merging targets is that, what if some plugin specifies the target as an array whereas another plugin supplies target as an object or a string.

Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo Oct 20, 2020

Choose a reason for hiding this comment

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

targets are only allowed in user config files, not in presets: presets need to be targets consumers.

For this reason, I tried thinking what conflicting targets could be caused from, and the most common use cases would be to have an env block that defines another set of targets for modern/legacy browsers.

For this reason, I think that replacing targets is what makes most sense: then both of these work:

{
  "targets": "> ie 10",
  "presets": ["@babel/env"],
  
  "env": {
    "modern": {
      "targets": { "esmodules": true }
    }
  }
}
{
  "targets": { "esmodules": true },
  "presets": ["@babel/env"],
  
  "env": {
    "legacy": {
      "targets": "> ie 10"
    }
  }
}

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 added these examples to the RFC body

@nicolo-ribaudo
Copy link
Member Author

babel/babel#8809

What should be the behavior of esmodules: true?

@babel/preset-env's esmodules overrides the browsers option, and it is overridden by explicit browser versions:

targets option resolved (only showing firefox)
{ browsers: "firefox 50" } firefox 50
{ browsers: "firefox 70" } firefox 70
{ esmodules: true } firefox 60
{ browsers: "firefox 50", esmodules: true } firefox 60
{ browsers: "firefox 70", esmodules: true } firefox 60
{ firefox: 50, esmodules: true } firefox 50
{ firefox: 70, esmodules: true } firefox 70

I think that "explicit browser versions win" is a good rule so that you can explicitly overwrite specific browsers in the browserslist/esmodules output.

However, I'm not sure about why esmodules should take precedence over browsers/browserslist: if someone wants to completely ignore the browsers option, they can just avoid setting it.

What if we computed the intersection of browsers and esmodules instead? The previous table would then became:

targets option resolved (only showing firefox)
{ browsers: "firefox 50" } firefox 50
{ browsers: "firefox 70" } firefox 70
{ esmodules: true } firefox 60
{ browsers: "firefox 50", esmodules: true } firefox 60, because older versions don't support es modules
{ browsers: "firefox 70", esmodules: true } firefox 70, because the user doesn't intend to support firefox versions older than 70
{ firefox: 50, esmodules: true } firefox 50
{ firefox: 70, esmodules: true } firefox 70

The main downside I see is that if someone uses a .browserslistrc file but want the current behavior of esmodules, then this config wouldn't work:

{
  "targets": { "esmodules": true }
}

because we are still loading and intersecting the .browserslistrc config file. They should use something like this instead:

{
  "browserslistConfigFile": false,
  "targets": { "esmodules": true }
}

However I'm note sure about why someone would want to support targets older than what they have in their .browserslistrc, so this is probably an edge case.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Oct 22, 2020

Regarding my previous comment about the esmodules, I discovered that Browserslist 4.13 introduced support for a new supports XXX target, and I tested how supports es6-module behaves:

browserslist query resolved (only showing firefox)
supports es6-module firefox 60
firefox >= 50, supports es6-module firefox 50
firefox >= 70, supports es6-module firefox 60
firefox >= 50 and supports es6-module firefox 60
firefox >= 70 and supports es6-module firefox 70

That is: by default (at least, the default behavior we provide) it computes the union of the targets, and you can specify and to compute the intersection.


Given that:

  • Browserslist now supports the equivalent of our esmodules option;
  • it provided users the choice between union and intersection with the other targets;
  • it's supports XXXX query is much more powerful than ours (for example, it can do supports webworkers or supports es6-generators)
  • our current esmodules: true will very soon start to bring in more transforms than just using a common browserslist query:
    targets: ">0.5%, not ie 11" esmodules: true
    chrome 80 chrome 61
    edge 18 edge 16
    firefox 74 firefox 60
    ios 12.2 ios 10.3
    safari 13 safari 10.1
    samsung 11.1 samsung 8.2

I'm leaning towards not including esmodules: true support in @babel/core. (see #2 (comment))

For people currently relying on it, it's a very simple change:

- targets: {
-   esmodules: true
- }
+ targets: "supports es6-module"

Thoughts?

@developit
Copy link
Member

Idea: if it's true that there are common browserslist queries that are more modern than esmodules:true, why not preserve a esmodules option and do an implicit union?

I'm still seeing the overwhelming majority of configurations specify a browserslist query that includes IE11, generally the default query of > 0.5%, last 2 versions, Firefox ESR, not dead:

and_chr 85+
and_ff 79+
and_qq 10.4+
and_uc 12.12+
android 81+
baidu 7.12+
chrome 84+
edge 85+
firefox 78+
ie 11
ios_saf 12.2+
kaios 2.5+
op_mini all
op_mob 59+
opera 70+
safari 13.1+
samsung 11.1+

@nicolo-ribaudo
Copy link
Member Author

After further discussion with @developit, we came to the conclusion that the esmodules shoud be maintained, and that it should be intersected with the Browserslist targets.

When using the module/nomodule pattern, it is important to have a single switch: they can then keep their complete targets in a .browserslistrc file so that it can be used by other tools, and use esmodules: true/false (which is only meaningful for Babel and not for other tools such as PostCSS) to generate the two different bundles.

Making it intersect the Browserslist targets rather than replacing them fixes the problem of described at babel/babel#8809.

@jquense
Copy link

jquense commented Mar 4, 2021

Making my way here from the trail of reading new features. Love the unified options. I see in the RFC the preset case with defined targets is brought up but doesn't really come to a good workaround? I don't think most instances of this issue are like CRA where the tool owns the entire build pipeline. For instance, we make heavy use of custom presets to enforce correct target configurations at our company. Providing a @my-company/babel-preset allows us to abstract away lot of potential footguns by automatically enforcing our companies set of support browsers. Now it seems like that isn't possible? Do the new options override targets set in presets, will they still be settable? Is there some other preferred way to share config like this?

@nicolo-ribaudo
Copy link
Member Author

Would something like https://babeljs.io/docs/en/options#extends work for you? The company provides a common config, and specific projects can extend it to add other plugins/presets.

@jquense
Copy link

jquense commented Mar 4, 2021

I guess the problem there is it's not easily parametrized, and relying on preset, plugin merging would be too coarse i think. That configs can be JS files means we could build some sort of config generator but that seems less than ideal, and doesn't compose nicely with the rest of ecosystem.

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

Successfully merging this pull request may close these issues.

10 participants