New bug in v3.4.2+3.4.3 with self-referencing variable function names like those used by d3.js

[michaeldrotar]

Bug report

*Uglify version 3.4.2 + 3.4.3

Apologies in advance cause I'm not really sure how to show a reproducible example, but this worked fine up to and including in v3.4.1.. broke in 3.4.2, still not working in 3.4.3 so I thought I should report it.

Our js stack includes d3.js, which has this code:

var polyIn = (function custom(e) {
  e = +e;

  function polyIn(t) {
    return Math.pow(t, e);
  }

  polyIn.exponent = custom;

  return polyIn;
})(exponent);

In 3.4.1, it compiled to this:

Av=function e(t){function n(e){return Math.pow(e,t)}return t=+t,n.exponent=e,n}(Ev)

Now it compiles to this:

Ev=function(t){function e(e){return Math.pow(e,t)}return t=+t,e.exponent=Oda,e}(Tv)

And produces an error that Oda is undefined. Minified names aside, can see it used to compile function e(t) and assigned that e to .exponent -- now just function(t) so nothing to assign. No idea where it's getting Oda from -- I can share more source if helpful but Oda is not referenced anywhere prior but is listed 21 times total (in our whole source, including outside of d3.js), but is never assigned a value nor used as a function name.

[ahorek]

lautis/uglifier#140

[alexlamsl]

@michaeldrotar what options were provided to uglify-js?

Practically there is nothing I can do until a reproducible test case can be devised, so this will eventually be closed unless you or somebody else come up with one.

[ahorek]

I just used default uglifier's options see https://github.com/lautis/uglifier
I believe this commit broke it #3193

[alexlamsl]

@ahorek to confirm your belief, try {compress:{arguments:false}} to see if the problem goes away.

[ahorek]

no, but {compress:{unused:false}} works

[ahorek]

here's a reproduction repo in ruby
https://github.com/ahorek/c3/tree/uglifiertest

generate_c3min.rb - generates c3min.js from c3.js
chart_bar_test.html - this should open a chart
c3_unused_false.js - already minified version (works)
c3_unused_true.js - already minified version (fails)

uglifier 4.1.14 (uglifyjs2 3.4.3) - doesn't work (works with {compress:{unused:false}})
uglifier 4.1.13 (uglifyjs2 3.4.2) - doesn't work (works with {compress:{unused:false}})
uglifier 4.1.12 (uglifyjs2 3.4.1) - and previous versions are ok

I'll try to reproduce it with uglifyjs only, but uglifier is just a ruby wrapper and the only thing that was changed between these versions was uglifyjs2

[alexlamsl]

@ahorek thanks for the effort - let me know when you have narrowed with only uglify-js on Node.js

[ahorek]

I wasn't able to reproduce it on nodejs, then I upgraded uglifier manually to uglifyjs 3.4.4 and it fixed the problem! #3207

thanks @alexlamsl

[alexlamsl]

@ahorek thanks for the testing - I can confirm that Uglifier turns on ie8 by default:

https://github.com/lautis/uglifier/blob/5a92d5c236c2101bb554f4f012122c5ef8d5487d/lib/uglifier.rb#L99

So it is a little more than just a wrapper 😉

[michaeldrotar]

Appreciate all the work, sorry I couldn't contribute more during the work day. Thanks!

[kzc]

An --ie8 bug was introduced in #3198 / ab36b9b for all browsers.

To reproduce the failure:

1. grab the files in this branch: https://github.com/ahorek/c3/tree/uglifiertest
2. run uglifyjs c3.js -bm --ie8 > c3min.js
3. open chart_bar_test.html in a modern browser to see a blank page and errors in the console.

If the same file is uglified with the same command with the uglify commit prior to ab36b9b, namely 2833091, then the web page works correctly and a chart can be seen.

[ahorek]

@kzc - sry, I can't reproduce your case with uglify-js 3.4.4

but maybe something is still wrong... I use uglifier with uglify-js 3.4.4.
ie8: true which is default in uglifier works as expected with both unused: true and unused: false. So my original problem was fixed, but if I force ie8: false it fails again.

Uglifier.compile(File.read("c3.js"), compress: {unused: true, ie8: false}))

here's a relevant minified snipet, I have an error ReferenceError: assignment to undeclared variable _c

  Y.tickValues = function (t) {
    if ('function' == typeof t) _c = function () {
      return t(W.domain())
    };
     else {
      if (!arguments.length) return _c;
      _c = t
    }
    return Y
  },
  Y

@alexlamsl you've a test case for this, it's wierd...
https://github.com/mishoo/UglifyJS2/pull/3198/files#diff-8ecafb30bcd28574b2af20c4ef3b9758R564

[kzc]

My mistake - the instructions in #3211 (comment) only lead to a reproducible failure in Safari 9, and not on latest Chrome and FireFox.

@ahorek What specific browser/version are you experiencing the error on?

[ahorek]

Lastest Firefox, but I don't think it's browser's fault. Is your error on Safari simmilar or something different?

I'ĺl retest it tomorrow.

[kzc]

The --ie8 flag should be deprecated. Not only does it have near zero market share, enabling it can be detrimental to other browsers.

[eight04]

enabling it can be detrimental to other browsers.

How? Is there a security issue or something?

[kzc]

Fixing ie8 bugs in mangle, compress and output for an obsolete browser with non-standard JS engine behavior can inadvertently introduce bugs for standards conforming browsers. ie8: true is not tested as rigorously as ie8: false.

[eight04]

It sounds like we should improve the test for ie8: true instead of removing it entirely.

[alexlamsl]

@eight04 ie8 won't be going away any time soon - you are welcomed to contribute to the testing efforts

And thanks for the relevant and detailed reports in the past - keep it up 👍