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

Uncaught ReferenceError: Invalid left-hand side expression in postfix operation on 2.8.0 #1516

Closed
malaman opened this issue Feb 28, 2017 · 48 comments · Fixed by #1519
Closed

Comments

@malaman
Copy link

malaman commented Feb 28, 2017

Hi,
we've received Uncaught ReferenceError: Invalid left-hand side expression in postfix operation on version 2.8.0 for all our js projects where we use uglify-js (webpack and browserify bundlers).

Do you have some other reports about this issue?

@pawelt
Copy link

pawelt commented Feb 28, 2017

Yes. It is broken. 2.7.5 still works though.

@jtormey
Copy link

jtormey commented Feb 28, 2017

Same issue here, was able to fix by setting compress: false in my webpack UglifyJsPlugin options.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

Please provide a repro.

@shiptoncraig
Copy link

👍 We're using grunt-contrib-uglify which is bringing this in automatically.

@pawelt
Copy link

pawelt commented Feb 28, 2017

We ended up adding uglify 2.7.5 to our package.json, which is then used by webpack, so everything works again.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

To fix the issue we'd need a short javascript program that exhibits the error when uglified. Please also provide the uglify options used.

@kargnas
Copy link

kargnas commented Feb 28, 2017

Sucks, we had this issue just now!

@graulund
Copy link

It's reproducible on Facebook's invariant.js:

https://github.com/zertosh/invariant/blob/master/invariant.js#L43

argIndex++ is uglified to 0++.

Via Webpack, we have our compress: { warnings: false } in our webpack.optimize.UglifyJsPlugin.

@alexlamsl
Copy link
Collaborator

@graulund I just ran uglify-js invariant.js -c warnings=false -b bracketize on that file and got:

"use strict";

var NODE_ENV = process.env.NODE_ENV, invariant = function(condition, format, a, b, c, d, e, f) {
    if ("production" !== NODE_ENV && void 0 === format) {
        throw new Error("invariant requires an error message argument");
    }
    if (!condition) {
        var error;
        if (void 0 === format) {
            error = new Error("Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings.");
        } else {
            var args = [ a, b, c, d, e, f ], argIndex = 0;
            error = new Error(format.replace(/%s/g, function() {
                return args[argIndex++];
            })), error.name = "Invariant Violation";
        }
        throw error.framesToPop = 1, error;
    }
};

I don't use Webpack, but have they specified some other compress options?

@baumanno
Copy link

baumanno commented Feb 28, 2017

A quick fix is to set reduce_vars: false and collapse_vars: false in the compress-object:

compress: {
    reduce_vars: false,
    collapse_vars: false
}

Relevant commits seem to be 4e49302 or 16cd5d5

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

Still not reproducible:

$ node_modules/.bin/uglifyjs -V
uglify-js 2.8.0

$ node_modules/.bin/uglifyjs invariant.js -c collapse_vars=false,reduce_vars=false
"use strict";var NODE_ENV=process.env.NODE_ENV,invariant=function(condition,format,a,b,c,d,e,f){if("production"!==NODE_ENV&&void 0===format)throw new Error("invariant requires an error message argument");if(!condition){var error;if(void 0===format)error=new Error("Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings.");else{var args=[a,b,c,d,e,f],argIndex=0;error=new Error(format.replace(/%s/g,function(){return args[argIndex++]})),error.name="Invariant Violation"}throw error.framesToPop=1,error}};module.exports=invariant;

$ echo $?
0

$ node_modules/.bin/uglifyjs invariant.js -c collapse_vars=true,reduce_vars=true
"use strict";var NODE_ENV=process.env.NODE_ENV,invariant=function(condition,format,a,b,c,d,e,f){if("production"!==NODE_ENV&&void 0===format)throw new Error("invariant requires an error message argument");if(!condition){var error;if(void 0===format)error=new Error("Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings.");else{var args=[a,b,c,d,e,f],argIndex=0;error=new Error(format.replace(/%s/g,function(){return args[argIndex++]})),error.name="Invariant Violation"}throw error.framesToPop=1,error}};module.exports=invariant;

$ echo $?
0

@graulund
Copy link

You're right, it's not reproducible for me over CLI either. It must be something in Webpack's UglifyJs wrapper, then?

@arijitdasgupta
Copy link

arijitdasgupta commented Feb 28, 2017

grunt-contrib-uglify had the same issue WebPack Uglify wrapper had.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

You're right, it's not reproducible for me over CLI either. It must be something in Webpack's UglifyJs wrapper, then?

That's a possibility. Some options may not be propagated using the lower level API.

No issue with the uglify minify() API AFAICT:

> require('uglify-js').minify('invariant.js', { compress: { reduce_vars: true, collapse_vars: true } })
{ code: '"use strict";var NODE_ENV=process.env.NODE_ENV,invariant=function(r,e,n,i,o,a,t,s){if("production"!==NODE_ENV&&void 0===e)throw new Error("invariant requires an error message argument");if(!r){var u;if(void 0===e)u=new Error("Minified exception occurred; use the non-minified dev environment for the full error message and additional helpful warnings.");else{var v=[n,i,o,a,t,s],d=0;u=new Error(e.replace(/%s/g,function(){return v[d++]})),u.name="Invariant Violation"}throw u.framesToPop=1,u}};module.exports=invariant;',
  map: null }

@bebraw
Copy link

bebraw commented Feb 28, 2017

@graulund Maybe it would be useful to debug against uglifyjs-webpack-plugin if someone can provide a small project. You can control Uglify version against that plugin (it's the same as in webpack core).

@retoheusser
Copy link

I had to fix this immediately for grunt-contrib-uglify which is also broken by this. I made use of https://docs.npmjs.com/cli/shrinkwrap

  1. run npm shrinkwrap --dev
  2. in the newly created npm-shrinkwrap.json go search for the explicit version 2.8.0 of uglify-js and change it manually to 2.7.5
  3. run npm install

@baumanno
Copy link

We are using grunt-contrib-uglify with only the drop_console-option set and were experiencing the issue with this snippet of code:

function createXMLHTTPObject() {
    var XMLHttpFactories = [];
    XMLHttpFactories.push(function () {
        return new XMLHttpRequest();
    });
    XMLHttpFactories.push(function () {
        return new ActiveXObject("Msxml2.XMLHTTP");
    });
    XMLHttpFactories.push(function () {
        return new ActiveXObject("Msxml3.XMLHTTP");
    });
    XMLHttpFactories.push(function () {
        return new ActiveXObject("Microsoft.XMLHTTP");
    });

    var xmlHttp = false;
    for (var i = 0; i < XMLHttpFactories.length; i++) {
        try {
            xmlHttp = XMLHttpFactories[i]();
        }
        catch (e) {
            continue;
        }

        break;
    }

    return xmlHttp;
}

Running it through grunt-contrib-uglify, we get this:

function b() {
    var a = [];
    a.push(function () {
        return new XMLHttpRequest
    }), a.push(function () {
        return new ActiveXObject("Msxml2.XMLHTTP")
    }), a.push(function () {
        return new ActiveXObject("Msxml3.XMLHTTP")
    }), a.push(function () {
        return new ActiveXObject("Microsoft.XMLHTTP")
    });
    for (; 0 < a.length; 0++) {
        try {
            !1 = a[0]()
        } catch (b) {
            continue
        }
        break
    }
    return !1
}

The issue resides in the for-loop, specifically 0++ (at least that's what I distilled...).

Apologies if this is the wrong place to comment and we should rather bug grunt-contrib-uglify, but I figured since you supply the functionality, you might want to be aware :-)

@maxired
Copy link

maxired commented Feb 28, 2017

The entry point of moment js is a good sample of the problem

// Moment prototype object
function Moment(config) {
    copyConfig(this, config);
    this._d = new Date(config._d != null ? config._d.getTime() : NaN);
    if (!this.isValid()) {
        this._d = new Date(NaN);
    }
    // Prevent infinite loop in case updateOffset creates new moment
    // objects.
    if (updateInProgress === false) {
        updateInProgress = true;
        hooks.updateOffset(this);
        updateInProgress = false;
    }
}

@alexlamsl
Copy link
Collaborator

@baumanno I cannot reproduce this on CLI:

$ uglifyjs test.js -c warnings=false -b bracketize
function createXMLHTTPObject() {
    var XMLHttpFactories = [];
    XMLHttpFactories.push(function() {
        return new XMLHttpRequest();
    }), XMLHttpFactories.push(function() {
        return new ActiveXObject("Msxml2.XMLHTTP");
    }), XMLHttpFactories.push(function() {
        return new ActiveXObject("Msxml3.XMLHTTP");
    }), XMLHttpFactories.push(function() {
        return new ActiveXObject("Microsoft.XMLHTTP");
    });
    for (var xmlHttp = !1, i = 0; i < XMLHttpFactories.length; i++) {
        try {
            xmlHttp = XMLHttpFactories[i]();
        } catch (e) {
            continue;
        }
        break;
    }
    return xmlHttp;
}

$ uglifyjs test.js -mc warnings=false -b bracketize
function createXMLHTTPObject() {
    var t = [];
    t.push(function() {
        return new XMLHttpRequest();
    }), t.push(function() {
        return new ActiveXObject("Msxml2.XMLHTTP");
    }), t.push(function() {
        return new ActiveXObject("Msxml3.XMLHTTP");
    }), t.push(function() {
        return new ActiveXObject("Microsoft.XMLHTTP");
    });
    for (var e = !1, n = 0; n < t.length; n++) {
        try {
            e = t[n]();
        } catch (t) {
            continue;
        }
        break;
    }
    return e;
}

... where test.js is from your example above.

Would be nice to know what additional options grunt-contrib-uglify supply on top of the default, or whether they have modified the input in some way?

@mishoo
Copy link
Owner

mishoo commented Feb 28, 2017

My guess is that it's the new reduce_vars option. Try setting that to false in the compressor options.

@alexlamsl I would advise defaulting that to false until it stabilizes... Can you roll out a new release with this?

@gaearon
Copy link

gaearon commented Feb 28, 2017

We have this error in Create React App end-to-end test suite. I can't give you the exact place where it errors because our logs are not verbose enough, but I suspect the earlier example with invariant is one of those places.

Our options:

      compress: {
        screw_ie8: true,
        warnings: false
      },
      mangle: {
        screw_ie8: true
      },
      output: {
        comments: false,
        screw_ie8: true
      },
      sourceMap: true

We also replace process.env.NODE_ENV with "production" before running Uglify.

Hope this helps somewhat!

@arijitdasgupta
Copy link

arijitdasgupta commented Feb 28, 2017

systemjs-builder has the same issue as well.

Looking into the code where they minify with Uglify... this is what they use to minify, using the API.

function minify(output, fileName, mangle, uglifyOpts) {
  var uglify = require('uglify-js');
  var ast;
  try{
    ast = uglify.parse(output.source, { filename: fileName });
  } catch(e){
    throw new Error(e);
  }
  ast.figure_out_scope();
  
  ast = ast.transform(uglify.Compressor(uglifyOpts.compress));
  ast.figure_out_scope();
  if (mangle !== false)
    ast.mangle_names();

  var sourceMap;
  if (output.sourceMap) {
    if (typeof output.sourceMap === 'string')
      output.sourceMap = JSON.parse(output.sourceMap);

    var sourceMapIn = output.sourceMap;
    sourceMap = uglify.SourceMap({
      file: fileName,
      orig: sourceMapIn
    });

    if (uglifyOpts.sourceMapIncludeSources && sourceMapIn && Array.isArray(sourceMapIn.sourcesContent)) {
      sourceMapIn.sourcesContent.forEach(function(content, idx) {
        sourceMap.get().setSourceContent(sourceMapIn.sources[idx], content);
      });
    }
  }

https://github.com/systemjs/builder/blob/f988373a164c9a5c868afc1e57664706d6f2ab79/lib/output.js#L74

This is probably a systemjs-builder issue, just posting it here for reference as how they are using the API.

@alexlamsl
Copy link
Collaborator

@mishoo I should probably do that, though I'm still scratching my head as to why I can't reproduce any of these using uglify-js along.

@gaearon
Copy link

gaearon commented Feb 28, 2017

Also, I just wanted to note that we all appreciate your amazing work on Uglify. Mishaps like this happen, but it’s vitally important that you folks keep innovating to make our builds smaller. ❤️

If someone got burned by this in production, they should use a lockfile.

@alexlamsl
Copy link
Collaborator

@maxired your example doesn't seem to break on CLI either:

$ uglifyjs test.js -c warnings=false -b bracketize
function Moment(config) {
    copyConfig(this, config), this._d = new Date(null != config._d ? config._d.getTime() : NaN),
    this.isValid() || (this._d = new Date(NaN)), updateInProgress === !1 && (updateInProgress = !0,
    hooks.updateOffset(this), updateInProgress = !1);
}

$ uglifyjs test.js -mc warnings=false -b bracketize
function Moment(e) {
    copyConfig(this, e), this._d = new Date(null != e._d ? e._d.getTime() : NaN), this.isValid() || (this._d = new Date(NaN)),
    updateInProgress === !1 && (updateInProgress = !0, hooks.updateOffset(this), updateInProgress = !1);
}

alexlamsl added a commit that referenced this issue Feb 28, 2017
@alexlamsl
Copy link
Collaborator

uglify-js 2.8.1 has been released with reduce_vars disabled by default.

Please test and report if there are any further issues.

@gaearon
Copy link

gaearon commented Feb 28, 2017

OK, I can confirm 2.8.1 fixed this for us. Thanks for very quick response!

@gaearon
Copy link

gaearon commented Feb 28, 2017

Can you npm deprecate [email protected] "There is a known issue, please update uglify-js to 2.8.1" to highlight this?

@arijitdasgupta
Copy link

Thanks a lot for the quick fix @alexlamsl

@alexlamsl
Copy link
Collaborator

@hinok thanks for the work - I'll look into it now.

@alexlamsl
Copy link
Collaborator

@gaearon npm deprecate [email protected] done

@cristian-sima
Copy link

It works
image

@rap2hpoutre
Copy link

Thank a lot for the quick fix, it works! Not sure it could help, but I found that the problem for my uglification was in a for loop:

Running for(var n=t.length;0<n;0++) {/* */} returns:

Uncaught ReferenceError: Invalid left-hand side expression in postfix operation

@cristian-sima
Copy link

cristian-sima commented Feb 28, 2017

@rap2hpoutre yep. The 0++ produces that. You can open the console and type 0++; and check the response, but I do not know why.

@danburzo
Copy link

Thank you for the quick turnaround! We got bitten by way of grunt-contrib-uglify as well, but luckily we had a yarn.lock file that was storing a workable shrinkwrap.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

I suspect that [email protected] with reduce_vars=true would work with grunt-contrib-uglify if this patch were applied to the grunt-contrib-uglify project:

diff --git a/tasks/lib/uglify.js b/tasks/lib/uglify.js
index 7a16066..97b74a1 100644
--- a/tasks/lib/uglify.js
+++ b/tasks/lib/uglify.js
@@ -98,7 +98,7 @@ exports.init = function(grunt) {
         options.compress.screw_ie8 = false;
       }
       var compressor = UglifyJS.Compressor(options.compress);
-      topLevel = topLevel.transform(compressor);
+      topLevel = compressor.compress(topLevel);
 
       // Need to figure out scope again after source being altered
       if (options.expression === false) {

Similarly, uglifyjs-webpack-plugin would likely work with [email protected] with the following patch to the uglifyjs-webpack-plugin project:

diff --git a/src/index.js b/src/index.js
index 9155801..92e6d5c 100644
--- a/src/index.js
+++ b/src/index.js
@@ -87,7 +87,7 @@ class UglifyJsPlugin {
                                                        const compress = uglify.Compressor(options.compress || {
                                                                warnings: false
                                                        }); // eslint-disable-line new-cap
-                                                       ast = ast.transform(compress);
+                                                       ast = compress.compress(ast);
                                                }
                                                if(options.mangle !== false) {
                                                        ast.figure_out_scope(options.mangle || {});

@bebraw Can you confirm this?

Edit: independently confirmed.

@alexlamsl
Copy link
Collaborator

@kzc Ah, I see what could go wrong there. webpack also seems to do that instead of .compress(ast).

@sppatel
Copy link

sppatel commented Feb 28, 2017

This broke us too.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

Compressor.compress() was originally introduced into Uglify to implement -c passes=N. It appears that reduce_vars later made use of it.

All upstream uglify-js wrappers should update their code accordingly.

I verified that if bin/uglifyjs were to use the same low level API as webpack and grunt-contrib-uglify then reduce_vars would exhibit the same problem as seen in this thread.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

minify() uses Compressor.compress() so it would not experience this problem. Nor would its downstream wrappers such as gulp-uglify.

@kzc
Copy link
Contributor

kzc commented Feb 28, 2017

Ideally all downstream uglify-js wrappers should use the Uglify minify() API rather than the low-level Uglify API.

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Feb 28, 2017
Modules like webpack and grunt-contrib-uglify still uses `ast.transform(compressor)` before `Compressor.compress(ast)` was introduced.

Workaround this compatibility issue by deactivating `reduce_vars` in such case.

fixes mishoo#1516
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Feb 28, 2017
Modules like webpack and grunt-contrib-uglify still uses `ast.transform(compressor)` before `Compressor.compress(ast)` was introduced.

Workaround this compatibility issue by deactivating `reduce_vars` in such case.

fixes mishoo#1516
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Feb 28, 2017
Modules like webpack and grunt-contrib-uglify still uses `ast.transform(compressor)` before `Compressor.compress(ast)` was introduced.

Workaround this compatibility issue by deactivating `reduce_vars` in such case.

Also fix use case with omitted `options` when calling `Compressor()`.

fixes mishoo#1516
alexlamsl added a commit that referenced this issue Feb 28, 2017
Modules like webpack and grunt-contrib-uglify still uses `ast.transform(compressor)` before `Compressor.compress(ast)` was introduced.

Workaround this compatibility issue by deactivating `reduce_vars` in such case.

Also fix use case with omitted `options` when calling `Compressor()`.

fixes #1516
@sarn3792
Copy link

sarn3792 commented Mar 2, 2017

How Can I update [email protected]. to [email protected]?

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.