[Preset-Env] Pass babel api.env() to browserslist to allow custom env settings

[swernerx]

...passing through the whole Babel infrastructure. This is useful for SSR and multi browser builds. Fixes: https://github.com/babel/babel/issues/8834

Q	A
Fixed Issues?	Fixes #8834
Patch: Bug Fix?	Yes
Major: Breaking Change?	No (for most users I figure)
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?	No
License	MIT

Previously we were not able to select an environment for browserslist when using parallel compilation like e.g. in Webpack with its multi-compiler infrastructure. Setting BROWSERSLIST_ENV is not actually feasible as it changes a global runtime specific settings and might break through the boundaries of the current Babel transpilation. What is more elegant - and I would think right - is to pass the environment which is used by Babel itself, which is received via api.env() to browserslist env option. This allows us to use Webpack babel-loader envName passing to Babel, which is available in preset-env and passed to browserslist.

This feature is often needed when doing multi target builds e.g. legacy and modern browsers, NodeJS and browser builds (for a library or SSR app), etc.

[babel-bot]

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

[ai]

@yavorsky or @existentialism may help here.

env is an important thing and was originally requested from Create React App.

[existentialism]

@swernerx LGTM, can we add a test?

[loganfsmyth]

Is there risk of this breaking existing configs if we start detecting environments we didn't before?

[ghost]

@existentialism I am actually not sure how to test this. But maybe that is mainly because I am not deeply involved into the Babel structure. A test would be best if it really is somewhat end-to-end e.g. webpack -> babel-loader -> preset-env -> browserslist.

[ghost]

@loganfsmyth it might behave differently (break if you will) for people who were using envName for Babel before. The new behavior will rank envName from Babel higher than BROWSERSLIST_ENV from previous implementation.

[existentialism]

@swernerx since we run tests with the test env, could we setup a fixture that has a browserslist config specific for that?

[swernerx]

@existentialism that should work. Browserslist defaults to production which could be added too - just to see that it correctly selects test.

[loganfsmyth]

.env() defaults to development, I'm worried that this addition would break people relying on it defaulting to production.

[yavorsky]

@swernerx I'm not sure we need to fully test Browserslist's part here. Just be sure we are passing everything and it's working the same way as before with default options.

[yavorsky]

Also wouldn't be confusing to have a situation: I have browserslsitrc for both styles and js. I'm running some build command with BROWSERSLIST_ENV='production' npm run build. I would expect both builds will use production config for browserslist, but actually js part will be for development and css - for production. Am I right?

[ghost]

@yavorsky Behavior depends on how we implement it... or Browserslist implement it. Actually the preference handling is completely on them. When they prefer the passed parameter over the environment... I figure that's correct - and their decision. I figure the same issue could be argued to be true for all these env variables... e.g. even the babel env falls back to NODE_ENV but is overridable by envName in Webpack. And it ignores the env in this case as well. I figure this just has to be documented.

[ghost]

@loganfsmyth In my opinion the problem is probably that all tools have some default values and these are not aligned. Even using development by default in Babel env() seems to be something at least worth discussing. We had a related discussion with a dotenv module I am the maintainer of. The argumentation chosen there was that fallback values are generally risky - and probably unexpected. That said, I would think that Babel env() would be better if it uses null when no value is set.

[ai]

Hm, taking env from NODE_ENV could be bad. Not every tool use NODE_ENV as Browserslist’s env, as result different tools will have different browsers

[loganfsmyth]

I think if we want to land this it would need to be off by default and toggled by an option. We can always try enabling it in a future major version, but it seems too risky for a minor version.

[ghost]

I implemented the suggested changes:

• Added a new option forwardEnv which passes the value of BABEL_ENV (=api.env()) to browserslist. It's a simple boolean value which is false by default.
• Added a fixture test which showcases the new functionality.
• No old tests were broken.
• The new tests produce different outputs based on the configuration in the new root .browserslistrc file.

[swernerx]

Sorry for the confusion with my two user names. Same person in different work contexts ;)

[ghost]

@loganfsmyth That's exactly what I implemented. Off by default.

[ai]

So it will use BABEL_ENV as Browerslist’s env? Can somebody explain me use case (sorry, I am not familiar with Babel very good) to check behavior consistency across other Browserslist’s tools?

[nicolo-ribaudo]

It's not just BABEL_ENV. It is also NODE_ENV and Babel's envName option. For example, if I programmatically call babel like this:

babel.transformSync(source, { envName: "production" }); 

it will forward "production" to browserlist and thus support more browsers, if browserlist's config is set to do so.

[swernerx]

Exactly @nicolo-ribaudo!

My use case is a little different though. For our SSR Webpack bundled app we bundle server and client in parallel during development and need to hit different browserslist configs for each of them. As it runs in parallel setting env-variables does not work reliable.

That said we have actually four different configs in browserslistrc: client-development, client-production, server-development and server-production. We pass these via envName on the babel-loader to Babel. With this fix in place this env is also passed over to browserslist.

[ai]

@nicolo-ribaudo @swernerx thanks. Perfect :+1:

[existentialism]

Should we make the test for this more isolated (don't universally set the env to env-from-preset, and keep the browserslist config directly inside fixture folder)?

[swernerx]

@loganfsmyth I updated the branch to current master and made the change you requested to pass the env over with envName - thanks for the feedback - much cleaner now.

[swernerx]

@nicolo-ribaudo Here you go: https://github.com/babel/website/pull/1935

[swernerx]

Should we make the test for this more isolated (don't universally set the env to env-from-preset, and keep the browserslist config directly inside fixture folder)?

I thought about this as well. Unfortunately it seems the browserslist config itself is only respected in the project root folder. It's documented as .browserslistrc config file in current or parent directories. - seems like the current directory means CWD which is probably the root of the test runner.

[swernerx]

Any chance of getting this merged?

[corneliusio]

God I need this…

[MilosRasic]

+1, need this for different builds targeting different platforms. For us finding out about this PR months later, is there anything we can do to help test or improve? autoprefixer already has this.

[ai]

I Autoprefixer 9.6 we deprecated browsers option (to force users to use Browserslist) and now Browserslist’s environment is the only way to have different build for modern and old browsers https://github.com/postcss/autoprefixer/issues/1222

This is why I think env option is very critical for Babel too. With this option we can do:

  "browserslist": {
    "modern": [
      "> 2%"
    ],
    "all": [
      "default"
    ]
  }

[ai]

People as env option from Babel to keep API in sync with Autoprefixer https://github.com/postcss/autoprefixer/issues/1222

[swernerx]

@loganfsmyth can we somehow get this thing merged. I mean it shouldn't really hurt anyone and offers so many benefits in our ecosystem.

[nicolo-ribaudo]

@existentialism I have moved the .browserslistrc file to the test folder, as you requested.

[bensampaio]

Any news on this?

[bensampaio]

@swernerx what's keeping this MR from being merged? Is there anything that can be done to help?

[AndrewLeedham]

Just ran into the issue that this addresses. Would be great to see in preset-env. Currently switching process.env.BROWSERSLIST_ENV forces builds to occur in sequence so they don't both change the global at the same time.

@swernerx any news on getting this merged :)

[swernerx]

I am out of options here. I am no maintainer of the project. Ping @loganfsmyth @existentialism ?

[AndrewLeedham]

I am out of options here. I am no maintainer of the project. Ping @loganfsmyth @existentialism ?

Seems like there is a merge conflict, perhaps that needs resolving then a maintainer can merge?

[swernerx]

Sure. That merge issue is pretty new. Still the PR was open for months before.

[JLHwung]

I am good with the idea of passing env to browserslists. However forwardEnv seems a little bit vague to me (what if one day preset-env depends other dev tools which has an env option?) and @Jessidhia's concern is also valid

I come up with a new design

browserslistEnv: string

As of Babel v7, the default value is "production", which is same as the default of browserslists, so it does not break anyone. And people can programmatically pass through api.env() in babel.config.js

["@babel/preset-env", { browserslistsEnv: api.env() }]

In Babel v8 we can change the defaults to be api.env(), and people can switch back to "production" if their app is indeed broken by this change.

cc @swernerx @nicolo-ribaudo

[ai]

I like browserslistsEnv too

[AndrewLeedham]

@swernerx thoughts on updating the PR with this approach. I can put a separate PR in for this if you don't have time to update it?