Merge pull request #5703 from NodeBB/uglify3

uglify-js@3, JS source maps finally supported

package.json

@@ -92,7 +92,7 @@
     "string": "^3.0.0",
     "templates.js": "0.3.10",
     "toobusy-js": "^0.5.1",
-    "uglify-js": "^2.6.0",
+    "uglify-js": "^3.0.11",
     "underscore": "^1.8.3",
     "underscore.deep": "^0.5.1",
     "validator": "7.0.0",

src/meta/js.js

@@ -88,6 +88,8 @@ module.exports = function (Meta) {
 		},
 	};
 
+	var basePath = path.resolve(__dirname, '../..');
+
 	function minifyModules(modules, fork, callback) {
 		var moduleDirs = modules.reduce(function (prev, mod) {
 			var dir = path.resolve(path.dirname(mod.destPath));
@@ -201,7 +203,10 @@ module.exports = function (Meta) {
 						};
 					});
 
-					moduleFiles = moduleFiles.concat(mods);
+					moduleFiles = moduleFiles.concat(mods).map(function (mod) {
+						mod.filename = path.relative(basePath, mod.srcPath).replace(/\\/g, '/');
+						return mod;
+					});
 
 					next();
 				});
@@ -287,8 +292,6 @@ module.exports = function (Meta) {
 				return callback(err);
 			}
 
-			var basePath = path.resolve(__dirname, '../..');
-
 			var scripts = Meta.js.scripts.base.concat(pluginScripts);
 
 			if (target === 'client' && global.env !== 'development') {
@@ -296,7 +299,11 @@ module.exports = function (Meta) {
 			}
 
 			scripts = scripts.map(function (script) {
-				return path.resolve(basePath, script).replace(/\\/g, '/');
+				var srcPath = path.resolve(basePath, script).replace(/\\/g, '/');
+				return {
+					srcPath: srcPath,
+					filename: path.relative(basePath, srcPath).replace(/\\/g, '/'),
+				};
 			});
 
 			callback(null, scripts);
@@ -315,12 +322,13 @@ module.exports = function (Meta) {
 			},
 			function (files, next) {
 				var minify = global.env !== 'development';
-
-				minifier.js.bundle(files, minify, fork, next);
-			},
-			function (bundle, next) {
 				var filePath = path.join(__dirname, '../../build/public', fileNames[target]);
-				fs.writeFile(filePath, bundle.code, next);
+
+				minifier.js.bundle({
+					files: files,
+					filename: fileNames[target],
+					destPath: filePath,
+				}, minify, fork, next);
 			},
 		], callback);
 	};

src/meta/minifier.js

@@ -1,18 +1,16 @@
 'use strict';
 
-var uglifyjs = require('uglify-js');
-var async = require('async');
 var fs = require('fs');
 var childProcess = require('child_process');
 var os = require('os');
+var uglifyjs = require('uglify-js');
+var async = require('async');
 var winston = require('winston');
 var less = require('less');
 var postcss = require('postcss');
 var autoprefixer = require('autoprefixer');
 var clean = require('postcss-clean');
 
-var file = require('../file');
-
 var Minifier = module.exports;
 
 function setupDebugging() {
@@ -163,13 +161,21 @@ function executeAction(action, fork, callback) {
 
 function concat(data, callback) {
 	if (data.files && data.files.length) {
-		async.mapLimit(data.files, 1000, fs.readFile, function (err, files) {
+		async.mapLimit(data.files, 1000, function (ref, next) {
+			fs.readFile(ref.srcPath, function (err, buffer) {
+				if (err) {
+					return next(err);
+				}
+
+				next(null, buffer.toString());
+			});
+		}, function (err, files) {
 			if (err) {
 				return callback(err);
 			}
 
 			var output = files.join('\n;');
-			callback(null, { code: output });
+			fs.writeFile(data.destPath, output, callback);
 		});
 
 		return;
@@ -179,81 +185,107 @@ function concat(data, callback) {
 }
 actions.concat = concat;
 
-function minifyJS(data, callback) {
+function minifyJS_batch(data, callback) {
-	if (data.batch) {
 	async.eachLimit(data.files, 1000, function (ref, next) {
 		var srcPath = ref.srcPath;
 		var destPath = ref.destPath;
+		var filename = ref.filename;
 
 		fs.readFile(srcPath, function (err, buffer) {
-				if (err && err.code === 'ENOENT') {
-					return next(null, null);
-				}
 			if (err) {
 				return next(err);
 			}
 
+			var scripts = {};
+			scripts[filename] = buffer.toString();
+
 			try {
-					var minified = uglifyjs.minify(buffer.toString(), {
+				var minified = uglifyjs.minify(scripts, {
-						// outSourceMap: data.filename + '.map',
+					sourceMap: {
-						compress: data.compress,
+						filename: filename,
-						fromString: true,
+						url: filename + '.map',
-						output: {
+						includeSources: true,
-							// suppress uglify line length warnings
-							max_line_len: 400000,
 					},
+					compress: false,
 				});
 
-					fs.writeFile(destPath, minified.code, next);
+				async.parallel([
+					async.apply(fs.writeFile, destPath, minified.code),
+					async.apply(fs.writeFile, destPath + '.map', minified.map),
+				], next);
 			} catch (e) {
 				next(e);
 			}
 		});
 	}, callback);
+}
+actions.minifyJS_batch = minifyJS_batch;
 
-		return;
+function minifyJS(data, callback) {
+	async.mapLimit(data.files, 1000, function (ref, next) {
+		var srcPath = ref.srcPath;
+		var filename = ref.filename;
+
+		fs.readFile(srcPath, function (err, buffer) {
+			if (err) {
+				return next(err);
 			}
 
-	if (data.files && data.files.length) {
+			next(null, {
-		async.filter(data.files, file.exists, function (err, scripts) {
+				srcPath: srcPath,
+				filename: filename,
+				source: buffer.toString(),
+			});
+		});
+	}, function (err, files) {
 		if (err) {
 			return callback(err);
 		}
 
+		var scripts = {};
+		files.forEach(function (ref) {
+			if (!ref) {
+				return;
+			}
+
+			scripts[ref.filename] = ref.source;
+		});
+
 		try {
 			var minified = uglifyjs.minify(scripts, {
-					// outSourceMap: data.filename + '.map',
+				sourceMap: {
-					compress: data.compress,
+					filename: data.filename,
-					fromString: false,
+					url: data.filename + '.map',
+					includeSources: true,
+				},
+				compress: false,
 			});
 
-				callback(null, minified);
+			async.parallel([
+				async.apply(fs.writeFile, data.destPath, minified.code),
+				async.apply(fs.writeFile, data.destPath + '.map', minified.map),
+			], callback);
 		} catch (e) {
 			callback(e);
 		}
 	});
-
-		return;
-	}
-
-	callback();
 }
 actions.minifyJS = minifyJS;
 
 Minifier.js = {};
-Minifier.js.bundle = function (scripts, minify, fork, callback) {
+Minifier.js.bundle = function (data, minify, fork, callback) {
 	executeAction({
 		act: minify ? 'minifyJS' : 'concat',
-		files: scripts,
+		files: data.files,
-		compress: false,
+		filename: data.filename,
+		destPath: data.destPath,
 	}, fork, callback);
 };
 
 Minifier.js.minifyBatch = function (scripts, fork, callback) {
 	executeAction({
-		act: 'minifyJS',
+		act: 'minifyJS_batch',
 		files: scripts,
-		batch: true,
 	}, fork, callback);
 };
 

test/build.js

@@ -20,12 +20,28 @@ describe('minifier', function () {
 	var scripts = [
 		path.resolve(__dirname, './files/1.js'),
 		path.resolve(__dirname, './files/2.js'),
-	];
+	].map(function (script) {
+		return {
+			srcPath: script,
+			destPath: path.resolve(__dirname, '../build/test', path.basename(script)),
+			filename: path.basename(script),
+		};
+	});
+
 	it('.js.bundle() should concat scripts', function (done) {
-		minifier.js.bundle(scripts, false, false, function (err, bundle) {
+		var destPath = path.resolve(__dirname, '../build/test/concatenated.js');
+
+		minifier.js.bundle({
+			files: scripts,
+			destPath: destPath,
+			filename: 'concatenated.js',
+		}, false, false, function (err) {
 			assert.ifError(err);
+
+			assert(file.existsSync(destPath));
+
 			assert.strictEqual(
-				bundle.code,
+				fs.readFileSync(destPath).toString(),
 				'(function (window, document) {' +
 				'\n\twindow.doStuff = function () {' +
 				'\n\t\tdocument.body.innerHTML = \'Stuff has been done\';' +
@@ -40,36 +56,40 @@ describe('minifier', function () {
 			done();
 		});
 	});
-
 	it('.js.bundle() should minify scripts', function (done) {
-		minifier.js.bundle(scripts, true, false, function (err, bundle) {
+		var destPath = path.resolve(__dirname, '../build/test/minified.js');
+
+		minifier.js.bundle({
+			files: scripts,
+			destPath: destPath,
+			filename: 'minified.js',
+		}, true, false, function (err) {
 			assert.ifError(err);
+
+			assert(file.existsSync(destPath));
+
 			assert.strictEqual(
-				bundle.code,
+				fs.readFileSync(destPath).toString(),
-				'(function(n,o){n.doStuff=function(){o.body.innerHTML="Stuff has been done"}})(window,document);function foo(n,o){return\'The person known as "\'+n+\'" is \'+o+" years old"}'
+				'(function(n,o){n.doStuff=function(){o.body.innerHTML="Stuff has been done"}})(window,document);function foo(n,o){return\'The person known as "\'+n+\'" is \'+o+" years old"}' +
+				'\n//# sourceMappingURL=minified.js.map'
 			);
 			done();
 		});
 	});
 
 	it('.js.minifyBatch() should minify each script', function (done) {
-		var s = scripts.map(function (script) {
+		minifier.js.minifyBatch(scripts, false, function (err) {
-			return {
-				srcPath: script,
-				destPath: path.resolve(__dirname, '../build/test', path.basename(script)),
-			};
-		});
-		minifier.js.minifyBatch(s, false, function (err) {
 			assert.ifError(err);
 
-			assert(file.existsSync(s[0].destPath));
+			assert(file.existsSync(scripts[0].destPath));
-			assert(file.existsSync(s[1].destPath));
+			assert(file.existsSync(scripts[1].destPath));
 
-			fs.readFile(s[0].destPath, function (err, buffer) {
+			fs.readFile(scripts[0].destPath, function (err, buffer) {
 				assert.ifError(err);
 				assert.strictEqual(
 					buffer.toString(),
-					'(function(n,o){n.doStuff=function(){o.body.innerHTML="Stuff has been done"}})(window,document);'
+					'(function(n,o){n.doStuff=function(){o.body.innerHTML="Stuff has been done"}})(window,document);' +
+					'\n//# sourceMappingURL=1.js.map'
 				);
 				done();
 			});