Don't make @babel/preset-env force all transforms in prod

[Lyrkan]

This PR removes the forceAllTransforms: webpackConfig.isProduction() line that is currently added to @babel/preset-env's options.

That setting was previously needed because UglifyJS did not support ES6 which meant that if you, for instance, had async functions in your code and targeted a recent browsers, they would have to be transformed for the minification process to be successful.

Nowadays we're using Terser which supports ES6, so there shouldn't be any issue keeping these kind of things in the final code anymore.

[Kocal]

A small comment, I think it can be good to notice it

[javiereguiluz]

👍 Nice! This will definitely be helpful. Thanks.

[weaverryan]

Oh my, I LOVE this. Encore exists in large part to smooth out the edges of the Webpack ecosystem for our users. However, whenever Webpack makes something better and we can remove hacks, it's wonderful. Thanks @Lyrkan!

[damienfa]

🤔 I've got a problem with that "fix".
The "browser support" is now different in Production than in dev / staging ....

For instance, my code contains "arrow functions", and my users have IE11. My "browserslistrc"
take into account IE11, and when I compile with Encore the arrow-functions are well converted to regular functions (thanks to @babel/preset-env, with corejs3 etc.).
But, if I compile for production with encore production, then, the arrow-functions are not replaced in the final build and IE11 users are stuck.

If I add back the line removed here, everything works fine.

It's a breakable change that have very bad consequences ...

I can technically add in my webpack.config.js the preset.forceAllTransforms = true thought the .configureBabel(...) function to make it works but I don't think it's not normal that the browser support and, more globally, the "transforms" differ from dev/staging to production.

[Lyrkan]

Hi @damienfa,

The "browser support" is now different in Production than in dev / staging ....

That was the case before that PR since Babel was applying additional transforms for production builds.

Now forceTransform should always have its default value (false) for both dev and prod builds, so Babel should generate the same code.

I did a quick test and cannot reproduce your issue locally with the latest version of Encore.

Using the following JS file:

const foo = () => {
  console.log('test');
};

foo();
foo();

And the following package.json file:

{
  "devDependencies": {
    "@symfony/webpack-encore": "^0.29"
  },
  "browserslist": [
    "last 2 Chrome versions"
  ]
}

I get this output:

// $ yarn encore dev
(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["main"],{

/***/ "./src/index.js":
/*!**********************!*\
  !*** ./src/index.js ***!
  \**********************/
/*! no static exports found */
/***/ (function(module, exports) {

const foo = () => {
  console.log('test');
};

foo();
foo();

/***/ })

// $ yarn encore prod
(window.webpackJsonp=window.webpackJsonp||[]).push([["main"],{tjUo:function(o,n){const t=()=>{console.log("test")};t(),t()}},[["tjUo","runtime"]]]);

Then if I change the package.json file:

{
  "devDependencies": {
    "@symfony/webpack-encore": "^0.29"
  },
  "browserslist": [
    "last 2 Chrome versions",
    "ie 11"
  ]
}

And I clear babel-loader's cache (required for dev builds, see this page) I get:

// $ yarn encore dev
(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["main"],{

/***/ "./src/index.js":
/*!**********************!*\
  !*** ./src/index.js ***!
  \**********************/
/*! no static exports found */
/***/ (function(module, exports) {

var foo = function foo() {
  console.log('test');
};

foo();
foo();

/***/ })

},[["./src/index.js","runtime"]]]);

// $ yarn encore prod
(window.webpackJsonp=window.webpackJsonp||[]).push([["main"],{tjUo:function(n,o){var t=function(){console.log("test")};t(),t()}},[["tjUo","runtime"]]]);

[damienfa]

OK. 🤔 You're right, I've checked with your test code in a brand new webpack project and I cannot reproduce my issue.

When I try with the same test code in my first environnement, it still fails : encore dev removes arrow-functions, encore production makes them back ... Then I've cleared the cache as you recommend, and it's solved 👍🏽 I cannot reproduce my issue anymore.

Moreover, I would want to add those 2 points :

• You say that "browser support" was already different on Production than on dev / staging because Babel was applying additional transforms for production builds. Right, I agree. But I didn't know that babel can remove transforms in production when they are set in dev. Are you sure about that ? I think it will be a bit dangerous ... (unless it's not explicitly set in the webpack.config.js with options or conditions)

• I cannot see your webpack.config.js file, but to make my "arrow-functions" transformed I'm forced to add preset.forceAllTransforms = true; in my .configureBabel(...) , is it the same to you ? Is it normal ?

Thanks,

[Lyrkan]

You say that "browser support" was already different on Production than on dev / staging because Babel was applying additional transforms for production builds. Right, I agree. But I didn't know that babel can remove transforms in production when they are set in dev. Are you sure about that ? I think it will be a bit dangerous ... (unless it's not explicitly set in the webpack.config.js with options or conditions)

I didn't said it removed things before, just that it didn't apply the same transforms in dev and prod. It basically added things that were not necessarily needed in prod builds in order to make UglifyJS (used before Terser) work properly.

That was not nominal since some optimizations can rely on recent ES features that were "fixed" even though your browserslist allowed them (for instance using arrow functions instead of standard functions).

I cannot see your webpack.config.js file, but to make my "arrow-functions" transformed I'm forced to add preset.forceAllTransforms = true; in my .configureBabel(...) , is it the same to you ? Is it normal ?

I didn't do anything special related to Babel in my webpack.config.js file, here it is:

const Encore = require('@symfony/webpack-encore');

Encore
  .enableSingleRuntimeChunk()
  .cleanupOutputBeforeBuild()
  .setOutputPath('build/')
  .setPublicPath('/')
  .cleanupOutputBeforeBuild()
  .addEntry('main', './src/index.js')
  .enableVersioning(Encore.isProduction())
;

module.exports = Encore.getWebpackConfig();

You shouldn't have to set forceAllTransforms to true, it works out of the box. If Babel does not transform your arrow functions it means that the browserslist it uses does not include targets that don't support them.