mishoo/UglifyJS

Compress option in 3.0.16 breaks how Webpack sets `global`

Closed this issue · 11 comments

Bug report or feature request?

Bug report

ES5 or ES6+ input?

ES5

Uglify version (uglifyjs -V)

3.0.16

JavaScript input

dist/bundle.js

/******/ (function(modules) { // webpackBootstrap
/******/ 	// The module cache
/******/ 	var installedModules = {};
/******/
/******/ 	// The require function
/******/ 	function __webpack_require__(moduleId) {
/******/
/******/ 		// Check if module is in cache
/******/ 		if(installedModules[moduleId]) {
/******/ 			return installedModules[moduleId].exports;
/******/ 		}
/******/ 		// Create a new module (and put it into the cache)
/******/ 		var module = installedModules[moduleId] = {
/******/ 			i: moduleId,
/******/ 			l: false,
/******/ 			exports: {}
/******/ 		};
/******/
/******/ 		// Execute the module function
/******/ 		modules[moduleId].call(module.exports, module, module.exports, __webpack_require__);
/******/
/******/ 		// Flag the module as loaded
/******/ 		module.l = true;
/******/
/******/ 		// Return the exports of the module
/******/ 		return module.exports;
/******/ 	}
/******/
/******/
/******/ 	// expose the modules object (__webpack_modules__)
/******/ 	__webpack_require__.m = modules;
/******/
/******/ 	// expose the module cache
/******/ 	__webpack_require__.c = installedModules;
/******/
/******/ 	// identity function for calling harmony imports with the correct context
/******/ 	__webpack_require__.i = function(value) { return value; };
/******/
/******/ 	// define getter function for harmony exports
/******/ 	__webpack_require__.d = function(exports, name, getter) {
/******/ 		if(!__webpack_require__.o(exports, name)) {
/******/ 			Object.defineProperty(exports, name, {
/******/ 				configurable: false,
/******/ 				enumerable: true,
/******/ 				get: getter
/******/ 			});
/******/ 		}
/******/ 	};
/******/
/******/ 	// getDefaultExport function for compatibility with non-harmony modules
/******/ 	__webpack_require__.n = function(module) {
/******/ 		var getter = module && module.__esModule ?
/******/ 			function getDefault() { return module['default']; } :
/******/ 			function getModuleExports() { return module; };
/******/ 		__webpack_require__.d(getter, 'a', getter);
/******/ 		return getter;
/******/ 	};
/******/
/******/ 	// Object.prototype.hasOwnProperty.call
/******/ 	__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };
/******/
/******/ 	// __webpack_public_path__
/******/ 	__webpack_require__.p = "";
/******/
/******/ 	// Load entry module and return exports
/******/ 	return __webpack_require__(__webpack_require__.s = 1);
/******/ })
/************************************************************************/
/******/ ([
/* 0 */
/***/ (function(module, exports) {

var g;

// This works in non-strict mode
g = (function() {
	return this;
})();

try {
	// This works if eval is allowed (see CSP)
	g = g || Function("return this")() || (1,eval)("this");
} catch(e) {
	// This works if the window reference is available
	if(typeof window === "object")
		g = window;
}

// g can still be undefined, but nothing to do about it...
// We return undefined, instead of nothing here, so it's
// easier to handle this case. if(!global) { ...}

module.exports = g;


/***/ }),
/* 1 */
/***/ (function(module, exports, __webpack_require__) {

/* WEBPACK VAR INJECTION */(function(global) {global.foo = "foo";
window.bar = "bar";

/* WEBPACK VAR INJECTION */}.call(exports, __webpack_require__(0)))

/***/ })
/******/ ]);

The uglifyjs CLI command executed or minify() options used.

uglifyjs --compress --beautify -- dist/bundle.js > dist/bundle.min.js

JavaScript output or error produced.

dist/bundle.min.js:

!function(modules) {
    function __webpack_require__(moduleId) {
        if (installedModules[moduleId]) return installedModules[moduleId].exports;
        var module = installedModules[moduleId] = {
            i: moduleId,
            l: !1,
            exports: {}
        };
        return modules[moduleId].call(module.exports, module, module.exports, __webpack_require__), 
        module.l = !0, module.exports;
    }
    var installedModules = {};
    __webpack_require__.m = modules, __webpack_require__.c = installedModules, __webpack_require__.i = function(value) {
        return value;
    }, __webpack_require__.d = function(exports, name, getter) {
        __webpack_require__.o(exports, name) || Object.defineProperty(exports, name, {
            configurable: !1,
            enumerable: !0,
            get: getter
        });
    }, __webpack_require__.n = function(module) {
        var getter = module && module.__esModule ? function() {
            return module.default;
        } : function() {
            return module;
        };
        return __webpack_require__.d(getter, "a", getter), getter;
    }, __webpack_require__.o = function(object, property) {
        return Object.prototype.hasOwnProperty.call(object, property);
    }, __webpack_require__.p = "", __webpack_require__(__webpack_require__.s = 1);
}([ function(module, exports) {
    var g;
    // [EDITED] This line is causing issue, and is different from what 3.0.15 produced
    g = this;
    try {
        g = g || Function("return this")() || (0, eval)("this");
    } catch (e) {
        "object" == typeof window && (g = window);
    }
    module.exports = g;
}, function(module, exports, __webpack_require__) {
    (function(global) {
        global.foo = "foo", window.bar = "bar";
    }).call(exports, __webpack_require__(0));
} ]);

If loaded in browser, foo is undefined.

Additional info

Compare with output using 3.0.15:

!function(modules) {
    function __webpack_require__(moduleId) {
        if (installedModules[moduleId]) return installedModules[moduleId].exports;
        var module = installedModules[moduleId] = {
            i: moduleId,
            l: !1,
            exports: {}
        };
        return modules[moduleId].call(module.exports, module, module.exports, __webpack_require__), 
        module.l = !0, module.exports;
    }
    var installedModules = {};
    __webpack_require__.m = modules, __webpack_require__.c = installedModules, __webpack_require__.i = function(value) {
        return value;
    }, __webpack_require__.d = function(exports, name, getter) {
        __webpack_require__.o(exports, name) || Object.defineProperty(exports, name, {
            configurable: !1,
            enumerable: !0,
            get: getter
        });
    }, __webpack_require__.n = function(module) {
        var getter = module && module.__esModule ? function() {
            return module.default;
        } : function() {
            return module;
        };
        return __webpack_require__.d(getter, "a", getter), getter;
    }, __webpack_require__.o = function(object, property) {
        return Object.prototype.hasOwnProperty.call(object, property);
    }, __webpack_require__.p = "", __webpack_require__(__webpack_require__.s = 1);
}([ function(module, exports) {
    var g;
    // [EDITED] This statement is compressed differently in 3.0.16
    g = function() {
        return this;
    }();
    try {
        g = g || Function("return this")() || (0, eval)("this");
    } catch (e) {
        "object" == typeof window && (g = window);
    }
    module.exports = g;
}, function(module, exports, __webpack_require__) {
    (function(global) {
        global.foo = "foo", window.bar = "bar";
    }).call(exports, __webpack_require__(0));
} ]);

If loaded in browser, foo is defined and equal to "foo".

The difference is towards the end:

// Original
g = (function() {
	return this;
})();

// Compressed with 3.0.16
g = this;

// Compressed with 3.0.15
g = function() {
        return this;
    }();

Source files used:

// index.js
global.foo = "foo";
window.bar = "bar";
// webpack.config.js
const path = require('path');

module.exports = {
  entry: './index.js',
  output: {
    path: path.resolve(__dirname, 'dist'),
    filename: 'bundle.js'
  }
};

package.json:

{
  "name": "uglify-bug",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "uglify-js": "3.0.16",
    "webpack": "2.6.1"
  }
}
kzc commented

@nicolashery Thanks for the very detailed bug report. We'll take a look.

kzc commented
"uglify-js": "3.0.16",
"webpack": "2.6.1"

This is not the version of uglify used by webpack@2.6.1 as it has its own uglify-js@2.x dependency:

    "uglify-js": "^2.8.27",

https://github.com/webpack/webpack/blob/v2.6.1/package.json#L24

As far as I know webpack needs an external plugin to use uglify-js@3 or uglify-es.

kzc commented

Cannot reproduce.

When I take the input from the bug report and feed it into uglify-js@3.0.16 I get the following results:

$ bin/uglifyjs -V
uglify-js 3.0.16
$ bin/uglifyjs webpack_bundle.js -c -b | tail -14
}([ function(module, exports) {
    var g;
    g = this;
    try {
        g = g || Function("return this")() || (0, eval)("this");
    } catch (e) {
        "object" == typeof window && (g = window);
    }
    module.exports = g;
}, function(module, exports, __webpack_require__) {
    (function(global) {
        global.foo = "foo", window.bar = "bar";
    }).call(exports, __webpack_require__(0));
} ]);

Re comment on Webpack, I'm actually using Webpack only to create the non-minified bundle. I use UglifyJS seperately to minify the Webpack output.

Re last comment, indeed it is the new compression that causes the issue:

// Original source
g = (function() {
  return this;
})();

// Compressed with 3.0.16
// Uglify seems to be inlining the results of the function now
// which causes the bug
g = this;

// Compressed with 3.0.15
// Uglify used to leave it untouched
g = function() {
  return this;
}();
kzc commented

See previous comment. Cannot reproduce using your JavaScript input dist/bundle.js.

I'm not sure I understand. If you take dist/bundle.min.js (produced by 3.0.16), and put it in an index.html like so:

<script src="dist/bundle.min.js"></script>

And open index.html in your browser. In the console of the browser, try for example foo.length you will get an error that foo is undefined.

If however you take the output produced by 3.0.15, you don't get an error trying to access the global foo.

kzc commented

I was looking at the wrong line. I now see the difference in output between 3.0.15 and 3.0.16:

$ git checkout v3.0.15
$ bin/uglifyjs dist/bundle.js -b -c | tail -14
    g = function() {
        return this;
    }();
    try {
        g = g || Function("return this")() || (0, eval)("this");
    } catch (e) {
        "object" == typeof window && (g = window);
    }
    module.exports = g;
}, function(module, exports, __webpack_require__) {
    (function(global) {
        global.foo = "foo", window.bar = "bar";
    }).call(exports, __webpack_require__(0));
} ]);
$ git checkout v3.0.16
$ bin/uglifyjs dist/bundle.js -b -c | tail -14
}([ function(module, exports) {
    var g;
    g = this;
    try {
        g = g || Function("return this")() || (0, eval)("this");
    } catch (e) {
        "object" == typeof window && (g = window);
    }
    module.exports = g;
}, function(module, exports, __webpack_require__) {
    (function(global) {
        global.foo = "foo", window.bar = "bar";
    }).call(exports, __webpack_require__(0));
} ]);
--- out.3.0.15.js	2017-06-14 20:54:43.000000000 -0400
+++ out.3.0.16.js	2017-06-14 20:54:32.000000000 -0400
@@ -30,9 +30,7 @@
     }, __webpack_require__.p = "", __webpack_require__(__webpack_require__.s = 1);
 }([ function(module, exports) {
     var g;
-    g = function() {
-        return this;
-    }();
+    g = this;
     try {
         g = g || Function("return this")() || (0, eval)("this");
     } catch (e) {

You can disable the compress option inline for the time being to get the same result as 3.0.15:

bin/uglifyjs dist/bundle.js -b -c inline=false
kzc commented

@alexlamsl I suspect that uglify shouldn't inline any function that contains this.

@kzc indeed not - interesting that doesn't come up in test/ufuzz.js

@nicolashery thanks for the report - working on a fix right now.

Thanks for the quick turnaround @kzc @alexlamsl! Version 3.0.17 fixed our build 😅