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

Use babel-minify instead of the old Uglify JS #15

Closed
morajabi opened this issue Dec 5, 2017 · 6 comments · Fixed by #157
Closed

Use babel-minify instead of the old Uglify JS #15

morajabi opened this issue Dec 5, 2017 · 6 comments · Fixed by #157

Comments

@morajabi
Copy link

morajabi commented Dec 5, 2017

https://github.com/babel/minify

Update: UglifyJS caused build to fail in this issue: #8

@devongovett
Copy link
Member

yep, only thing I was worried about is performance. last time I tried, it was quite a bit slower than uglify.

@morajabi
Copy link
Author

morajabi commented Dec 5, 2017

@devongovett I have no experience with that really! Just wanted to suggest it. I'm happy to investigate this if you wish.

@morajabi morajabi changed the title Use bable-minify instead of the old Uglify JS Use babel-minify instead of the old Uglify JS Dec 5, 2017
@elijahdorman
Copy link

elijahdorman commented Dec 5, 2017

In my current project, Babili support is fairly important due to ES5 and ES6 support (uglify gave us some ES6 issues, so we swapped).

All current comparisons I can find aren't really apples to apples comparisons. Parsing ES6 is a lot more complicated than parsing ES5. More parser code and more complex parsing are going to have an effect on speed. Every test I've ever seen compiles the code to ES5 and then runs Uglify. The issue is that when we parse the ES5, Uglify uses the happy ES5 paths while Babili still uses the full ES6 parser built into Babel.

Another factor is total parsing. When you use Babel, you can parse once then use that same tree for everything -- including minification. When you use UglifyJS, you parse using Babel and then throw that away only to parse again with Uglify. That seems like a lot of duplication.

I'm not sure how much that difference is in reality (seems a wash in our project), but I think someone needs to test both against the whole system and see the results rather than focusing in on one small, unrepresentative part of the process.

From a config perspective, our system got a bit more simple when we switched to Babel completely as we could get rid of our uglify config section and simply add a couple lines in our babelrc.

A final issue is importing non-JS assets inside JS files. It's not completely an issue per-se, but you need a plugin to remove non-JS imports in Babel so you can compile into the http2 friendly multiple files with ES6 format.

My current project has been heavily considering adding an assets.js to each folder with imports separate from the main JS. At that point, we simply run/output a babel compilation then run webpack against our assets.js files to load the rest. No double-parsing and we get to keep our asset pipeline more or less as-is.

@benjamn
Copy link

benjamn commented Dec 6, 2017

I am one of the maintainers of Meteor, and we began using Babili/babel-minify as a fallback for UglifyJS, back when https://www.npmjs.com/package/uglify-js didn't really support ECMAScript 2015+ syntax, and would often fail when it encountered newer syntax: meteor/meteor#8397, meteor/meteor#8519

Since then, the ECMAScript support of https://www.npmjs.com/package/uglify-es has improved significantly, so we use Babili much less often than before. Babili is still a useful fallback, but I would never, ever promote it to be the primary minifier for Meteor, because it uses an incredible amount of memory for medium to large files, and can be extremely slow (compared to uglify-es).

Here's the most recent bug report (from 5 days ago) from someone whose meteor deploy command failed because Babili gobbled up all available memory: meteor/meteor#9430

Although we limit the damage Babili can do by giving UglifyJS a chance to minify the code first, we still receive a steady trickle of bug reports from developers who fall into the Babili fallback and suffer the consequences. I understand that ECMAScript 2015+ support is more complicated than ES5, but Babili's performance is unreasonably bad.

In short: use Babili as a fallback for uglify-es, but please don't switch to it blindly.

@devongovett
Copy link
Member

Haven't looked into the babel-minify issue, but one thing that might help with memory usage is that Parcel currently minifies each file separately so we won't feed a giant bundle into the minifier all at once. This should speed things up quite a bit too.

Currently, Parcel tries to avoid code generating and re-parsing before running uglify by converting the Babel AST to an Uglify AST. That's the cause of the bug in #8 though, because the AST transform doesn't support some of babel's more advanced node types.

It would be easier to just use babel-minify, but if there are issues with it I hesitate. But worth trying out both babel-minify and uglify-es and doing some comparison.

@jamiebuilds
Copy link
Member

I would also hesitate to make it the default at this point. An option sure

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

Successfully merging a pull request may close this issue.

5 participants