IE8 flag + mangle w/ locals with the same name as functions is causing hoisting issues in IE8

[hilts-vaughan]

Bug report or feature request?

Given the following snippet:

var noop = function() {};
var define = function(mname, deps, cb) {
  cb(noop);
};

define('xxx/yyyy', ["module","moduleB","moduleC","moduleD"], function(require) {
  'use strict';
  
  var NamespaceForCode = {};
  NamespaceForCode.Scrollable = function Scrollable() {
  Scrollable.prototype.callMeHopefully = function callMeHopefully(argCall) {
    alert(argCall);
  };
  Scrollable.prototype.goingToFail = function goingToFail() {
    var goingToFail = {a: 1};
    return this.callMeHopefully(goingToFail);
  };
}

  var test = new NamespaceForCode.Scrollable();
  test.goingToFail();
});

Uglify version (uglifyjs -V)

3.5.2

JavaScript input

./node_modules/terser/bin/uglifyjs -b --mangle --ie8 -- ~/Desktop/Scrollable.js

^ with the above input

JavaScript output or error produced.

var noop = function() {};

var define = function(o, l, e) {
    e(noop);
};

define("xxx/yyyy", [ "module", "moduleB", "moduleC", "moduleD" ], function(o) {
    "use strict";
    var l = {};
    l.Scrollable = function n() {
        n.prototype.callMeHopefully = function o(l) {
            alert(l);
        };
        n.prototype.goingToFail = function l() {
            var l = {};
            return this.callMeHopefully(l);
        };
    };
    var e = new l.Scrollable();
    e.goingToFail();
});

You will note the named function has the same name as the local. This causes issues and mixes up what this is in the context of the function since JScript bugs 😱

http://kangax.github.io/nfe/#jscript-bugs

[alexlamsl]

Your example won't work under IE8

❌"Object doesn't support this property or method"

[hilts-vaughan]

I was not clear in the example since I typed it from memory but it seems you have figure out the issue from above, right? :)

It's the shadowing from the JScript engine that is causing the problems.

[alexlamsl]

I found one issue which I've made a Pull Request to fix, but I'm not sure if that's your actual issue though.

It would be helpful if you can confirm the changes in #3356 works for your actual codebase.

[hilts-vaughan]

Just checked and it didn't seem to. To be clear, all I did was clone the repo, apply the patch from their PR and then replace the folder in my node_modules w/ this one. I will double check this is the same thing.

The issue I am observing boils down to the following:

1. I have an AMD module with define wrapper around it. The first argument to callback for define, is require. It is assigned the minified identifier e
2. I have some method inside of a Babelfied class. Let's call it f. Inside of f is a local with the same name, f
3. When minifying, the function is converted to a named function with the letter e sometimes. And then the local has the name e. The code looks something like this...

define("Scrollable", [ "something" ], function(e, t, n) {
    var a = e("something")
    l.Scrollable = function(e) {
        function t() {
        }
        return babelHelpers.inherits(t, a), t.prototype.events = function e() {
            var e = {};
           // do something with `this`
          this.getOption(...) // declared on the super class

The issue is that e becomes the method below. It ends up not returning the module at all. Somehow, in IE, the scoping is becoming out of whack. The only points in the code this happens is where I have a local in a method w/ the same name. This wasn't happening on older versions of UglifyJS either -- it's only when I upgraded. I can try performing a bisect if this is not enough information for you to go off.

I've worked around this for now by just renaming the locally scoped variables inside of the methods to something other than what the identifier of the method name is. Once you do that, things minify fine and there is no problem.

Is there anything else I can do for you? I can try and par it down into something more runnable as well but since it's an enterprise application, I'll probably need to see if if I can create a stripped down example in a repo somewhere... since I can't just link the entire code base.

[alexlamsl]

What I'm saying is that your original, unmangled example doesn't work in IE8, and for the same reason as well.

So what I need is an example that:

• works as expected under IE8
• stops working after --mangle --ie8

Take the new test cases in #3356, before the fix the mangled output would have been false instead of true.

[hilts-vaughan]

OK, I setup an example repo for you here. https://github.com/hilts-vaughan/uglifyjs-ie8-bug-demonstration

I did the following..

1. Cloned the repo
2. Ran python -m SimpleHTTPServer to spin up a server
3. Under IE7, I visited index.html. It worked (non-minfiied code)
4. I ran build.sh to output an IE compliant minfied files
5. I visited index.min.html and it failed

I am using IE11 compat. mode here for the better debugger -- but the result is the same, compatibility or not.

For the record, it works fine in version npm install uglify-js@2.6.4 && ./build.sh but then fails on npm install uglify-js@2.7.0 && ./build.sh However, I did notice in version 2.7.0 that screw-ie was introduced, so it could just be an artifact of that. I would need to run more bisection tests past 2.7.0 to pinpoint a specific version.

Do you need more info than this or is this enough for you to go on?

[hilts-vaughan]

Please be advised you can find the broken output here:

https://github.com/hilts-vaughan/uglifyjs-ie8-bug-demonstration/blob/master/app.min.js

You can find the output of 2.6.4 here: https://github.com/hilts-vaughan/uglifyjs-ie8-bug-demonstration/blob/master/app.old.min.js

You can note the differences are mostly in the variable name(s) assigned to the method in Scrollable
The older version of Uglify used unique names. The bug itself manifests from the fact that require from the outer context ends up importing some completely different module.

If you have questions, let me know..

[alexlamsl]

Thanks a lot for the hard work!

I'll have a look this weekend and track down the root cause.

[hilts-vaughan]

Great, thank you! Let me know if you need any more info.

[alexlamsl]

Turns out the Pull Request already fix your test case as well − in any case, we now have a new test case 😉

[hilts-vaughan]

Interesting! I tried applying the patch to the entire code base before and couldn't get it to work -- but it was using grunt-contrib-uglifyjs and I might not have modified the file(s) properly.

Could you publish a new version to NPM and I will test? :)

[alexlamsl]

Sure, will do.

[hilts-vaughan]

I can confirm w/ the latest version that my project is building and passing testing under IE again.

Thank you for your hard work!

[alexlamsl]

Thank you for the helpful details and testing confirmation 😉