search

ctrlplusb / react-universally

A starter kit for universal react applications.
MIT License
1.7k stars 244 forks source link

Issues with uglifyJS #514

Open oyeanuj opened 6 years ago

oyeanuj commented 6 years ago

I ran into the following messages when I was building a production bundle for my branch here: https://github.com/oyeanuj/react-universally/tree/redux-styling-data-fetching

ERROR in index-53d0c3e8c25a725bf591.js from UglifyJs
Unexpected token: keyword (const) [index-53d0c3e8c25a725bf591.js:29064,5]

From Googling it around, it was common with Uglify v2 and ES6 code. There are recommendations to switch to this Babel-Minify plugin for Webpack.

So, my question is if someone else has seen this issue, and/or if we should switch to the Babel-Minify Webpack plugin?

Thought anyone? cc: @ctrlplusb @diondirza

codepunkt commented 6 years ago

I've ran into the same problem on other projects before, couldn't figure out what went wrong and have started to exclusively use babel-minify - so far without further problems.

oyeanuj commented 6 years ago

@codepunkt Interesting, do you think it is PR-able to this repo? If not, can you share your babel-minify config and setup options? Thank you!

ctrlplusb commented 6 years ago

I think the issue is that there is some code not being allowed to be parsed by our babel config. babel-minify looks like it will allow es6 code to be transpiled and therefore the minified code will only work on modern browsers.

oyeanuj commented 6 years ago

@ctrlplusb What solution would you recommend here?

Also, FWIW, babel-minify README says the that we could use it for older browsers as well if we transpile down first:

Babel Minify is best at targeting latest browsers (with full ES6+ support) but can also be used with the usual Babel es2015 preset to transpile down the code first.

oyeanuj commented 6 years ago

@codepunkt Just pinging to check if you could share your babel-minify setup (or even PR it)? I'd super appreciate that since I keep running into issues with various different packages thanks to Uglify and am looking to switch!

oyeanuj commented 6 years ago

An update: So, it turns out that Webpack v3 so far uses the 0.4.6 of the UglifyJS plugin and with v4, they will start using 1.x of the UglifyJS plugin (which internally uses Uglify-ES).

So, I upgraded to v1 and the errors seem to go away. Incase, anyone needs to do the same, see commit

I tried Babel-Minify as well and I was getting the same bundle size but slower builds so haven't gone that route yet.

The only caveat to all of this is that I haven't tested my builds in IE 11 yet, so not sure how it might work there. But given that, Webpack v4 is going to default to Uglify Plugin's v1, we should consider if we need to upgrade, or switch to babel-minify.

The ideal holy grail solution to be able to deliver modern code to modern browsers and ES5 code to older browsers seems to be described here - https://philipwalton.com/articles/deploying-es2015-code-in-production-today. That in v14 could be a good target!

cc: @ctrlplusb @strues