[next] Revert to use ecma 5 in uglifyOptions

[danielberndt]

Building my app using the [next] version of react-scripts lead to issues when trying to open the minified results on IE11.

It's a combination of using the commonmark.js library and using the {ecma: 8} in the uglifyOptions. That led to unicode characters in the output that can't be parsed by IE11. Also the Google crawler bot is yelling Unexpected token ILLEGAL at the resulting code.

Look here for another discussion about the same issue: rails/webpacker#1235

[Timer]

This will impact parsing of newer browser syntax, no? I believe #4221 is related.

There is probably an uglify option we can shut off to disable this behavior -- I'd rather do that instead.

[danielberndt]

Ah, yes, a closer look into the docs reveals that you can place the ecma option for each of parse, compress, and output. So I believe what we want is something like

{
  parse: {ecma: 8,...},
  compress: {ecma: 5,...},
  output: {ecma: 5,...},
}

which are the default values already. So I think the simplest thing is to not have any explicit ecma setting on our end.

[Timer]

Output is not necessarily going to be ecma: 5 though -- it's whatever browser you target.

Can you provide a sample output of something using newer features against this setting? e.g. 5 ** Math.PI.

[Timer]

Oh, I just toyed with this and it appears ecma: 5 still outputs ES5+ syntax (weird).

Let's be explicit and do to prevent any upstream changes by accident:

{
  parse: {ecma: 8,...},
  compress: {ecma: 5,...},
  output: {ecma: 5,...},
}

And add an explanation for why the values are as such (and a link to this issue preceding it all).

[danielberndt]

Alright, done :)

[Timer]

Perfect, thanks for this!

[gaearon]

"why"

[gaearon]

I'm a bit confused. Didn't we want to let people ship uncompiled code in production too if they don't care about older browsers per their browserlist?

[danielberndt]

uglify-es is currently not configured to consider the browserlist settings.
The previous settings lead to code that couldn't be parsed on old browsers, no matter what you put in your browserlist.

Also: this setting won't transpile your code down to es5, it will just avoid minification steps that only work for es6+.

For example: an ecma setting of 5 will not convert ES6+ code to ES5.
https://www.npmjs.com/package/uglify-es

[gaearon]

Thanks for explaining.

[Timer]

Yeah, it's a little counter-intuitive -- setting output: 5 still outputs ES6+ code/syntax -- this really confused me at first (it only disables es6+ optimizations).
I figure this is the best solution until uglify respects browserslist.

@danielberndt did you want to send a follow up to tweak the docs / fix that "why"?

Also, how about we open an issue with uglify about supporting browserslist?

[danielberndt]

Sure, will put a short version of my explanation as a code comment :)

[danielberndt]

Ah now I see! The "why" was referring to a typo, and not a question 😅

[kopax]

Hi guys, could we release a fix for v2?

[mnemanja]

I'd appreciate this as well. When can it be expected?

[danielberndt]

It's already released :)

you can check it here: https://unpkg.com/react-scripts@2.0.0-next.66cc7a90/config/webpack.config.prod.js

[Timer]

Be sure you always install react-scripts@next --exact; never trust upgrade on a caret range.