Rich-Harris / butternut

The fast, future-friendly minifier
https://butternut.now.sh
MIT License
1.17k stars 17 forks source link

compare with uglify #97

Closed Rich-Harris closed 7 years ago

Rich-Harris commented 7 years ago

This adds a warning if Uglify does a better job of compressing a test sample than Butternut. In some cases Uglify fails (async, anonymous function declaration default exports), and in some cases it generates bad output (class a||b{}), but in many cases it's just doing a better job, and we should steal those optimisations.

A few examples:

Removing unused function expression IDs

⚠️   uglify-es generated smaller output:
      butternut: var foo=function a(b,c){console.log(b);console.log(c)}
      uglify-es: var foo=function(o,l){console.log(o),console.log(l)};

Removing empty IIFEs

⚠️   uglify-es generated smaller output:
      butternut: !function(){}()
      uglify-es:

Writing Infinity as 1/0

⚠️   uglify-es generated smaller output:
      butternut: x=Infinity
      uglify-es: x=1/0;

Inlining certain values

⚠️   uglify-es generated smaller output:
      butternut: function foo(){var a=1,b=2;console.log(a,b)}
      uglify-es: function foo(){console.log(1,2)}

Inlining function calls and removing redundant return statements

⚠️   uglify-es generated smaller output:
      butternut: function foo(){a();return;function a(){console.log('bar')}}
      uglify-es: function foo(){!function(){console.log("bar")}()}

Not parenthesizing assignment expressions

⚠️   uglify-es generated smaller output:
      butternut: foo?(a=b):(a=c)
      uglify-es: foo?a=b:a=c;

Inlining return values from functions

⚠️   uglify-es generated smaller output:
      butternut: function x(){var a=function(){return 42};if(y){var b=a();console.log(b)}}
      uglify-es: function x(){if(y){console.log(42)}}

Optimising if(a)return b;else return c

⚠️   uglify-es generated smaller output:
      butternut: function foo(){if(a)return b;else return c}
      uglify-es: function foo(){return a?b:c}

Removing parens in NewExpression without parameters

⚠️   uglify-es generated smaller output:
      butternut: new Foo()
      uglify-es: new Foo;

There's a couple of others but those are the main ones.

codecov[bot] commented 7 years ago

Codecov Report

Merging #97 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #97   +/-   ##
=======================================
  Coverage   87.36%   87.36%           
=======================================
  Files          71       71           
  Lines        1527     1527           
=======================================
  Hits         1334     1334           
  Misses        193      193

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4826591...387755f. Read the comment docs.

kzc commented 7 years ago

In some cases Uglify fails (async, anonymous function declaration default exports), and in some cases it generates bad output (class a||b{})

ES2017 is not yet supported in uglify: https://github.com/mishoo/UglifyJS2/issues/1789

Just created the PR https://github.com/mishoo/UglifyJS2/issues/1954 for exporting default anonymous functions and classes.

What input leads to (class a||b{})?

It'd be great if you'd create uglify issues as you find bugs. We're all friends here.

Rich-Harris commented 7 years ago

It'd be great if you'd create uglify issues as you find bugs. We're all friends here.

I mean to — just trying to work through some of the low-hanging fruit first. Sorry, trying to juggle too many things!

What input leads to (class a||b{})?

Typo — I meant class foo extends a || b {}. It's possible for a superclass to be an expression, but in most cases (anything below 18 in the operator precedence table) it needs to be parenthesized:

// valid
class foo extends (a||b) {}

// invalid
class foo extends a||b {}

Uglify is removing the parentheses (I only found this via eslump — highly recommended if you're not already using something like it).

kzc commented 7 years ago

Thanks!

I'll have to check out eslump!

kzc commented 7 years ago

Uglify is removing the parentheses

More accurately since Uglify parses to an AST, it's not adding the parentheses. :-)

It's a different perspective with a magic-string based approach.

OT - Buble hit a technical snag related having multiple transforms operate on the same object literal in a few tickets. Only so much inserting/deleting/removing/moving you can do at a single character position. See: https://gitlab.com/Rich-Harris/buble/issues/182 - I'm aware of a fix with a preprocessor pass to add parens around object literals, but it would not be pretty and would screw up source mappings.

Rich-Harris commented 7 years ago

since Uglify parses to an AST, it's not adding the parentheses. :-)

True!

OT - Buble hit a technical snag related having multiple transforms operate on the same object literal in a few tickets

Rats. Will take a poke soon.