Gracefully handle missing uglify-js dependency

nknapp · nknapp · commit d5caa56d6cb9 · 2017-10-17T22:17:37.000+02:00
closes #1391 uglify-js is an optional dependency and should be treated as such. This commit gracefully handles MODULE_NOT_FOUND errors while loading uglify. - Check for existing uglify-js (and load uglify-js) only if minification was activated - Use "require.resolve" to check if uglify exists. Otherwise, a missing dependency of uglify-js would cause the same behavior as missing uglify-js. (Only a warning, no error) - The code to load and run uglify is put into a single for readability purposes - Tests use a mockup Module._resolveFilename to simulate the missing module. This function is used by both "require" and "require.resolve", so both are mocked equally.
diff --git a/lib/precompiler.js b/lib/precompiler.js
@@ -4,7 +4,7 @@ import fs from 'fs';
 import * as Handlebars from './handlebars';
 import {basename} from 'path';
 import {SourceMapConsumer, SourceNode} from 'source-map';
-import uglify from 'uglify-js';
+
 
 module.exports.loadTemplates = function(opts, callback) {
   loadStrings(opts, function(err, strings) {
@@ -235,7 +235,6 @@ module.exports.cli = function(opts) {
     }
   }
 
-
   if (opts.map) {
     output.add('\n//# sourceMappingURL=' + opts.map + '\n');
   }
@@ -244,12 +243,7 @@ module.exports.cli = function(opts) {
   output.map = output.map + '';
 
   if (opts.min) {
-    output = uglify.minify(output.code, {
-      fromString: true,
-
-      outSourceMap: opts.map,
-      inSourceMap: JSON.parse(output.map)
-    });
+    output = minify(output, opts.map);
   }
 
   if (opts.map) {
@@ -271,3 +265,33 @@ function arrayCast(value) {
   }
   return value;
 }
+
+/**
+ * Run uglify to minify the compiled template, if uglify exists in the dependencies.
+ *
+ * We are using `require` instead of `import` here, because es6-modules do not allow
+ * dynamic imports and uglify-js is an optional dependency. Since we are inside NodeJS here, this
+ * should not be a problem.
+ *
+ * @param {string} output the compiled template
+ * @param {string} sourceMapFile the file to write the source map to.
+ */
+function minify(output, sourceMapFile) {
+  try {
+    // Try to resolve uglify-js in order to see if it does exist
+    require.resolve('uglify-js');
+  } catch (e) {
+    if (e.code !== 'MODULE_NOT_FOUND') {
+      // Something else seems to be wrong
+      throw e;
+    }
+    // it does not exist!
+    console.error('Code minimization is disabled due to missing uglify-js dependency');
+    return output;
+  }
+  return require('uglify-js').minify(output.code, {
+    fromString: true,
+    outSourceMap: sourceMapFile,
+    inSourceMap: JSON.parse(output.map)
+  });
+}
diff --git a/spec/precompiler.js b/spec/precompiler.js
@@ -12,6 +12,8 @@ describe('precompiler', function() {
 
   var log,
       logFunction,
+      errorLog,
+      errorLogFunction,
 
       precompile,
       minify,
@@ -26,16 +28,51 @@ describe('precompiler', function() {
       content,
       writeFileSync;
 
+  /**
+   * Mock the Module.prototype.require-function such that an error is thrown, when "uglify-js" is loaded.
+   *
+   * The function cleans up its mess when "callback" is finished
+   *
+   * @param {Error} loadError the error that should be thrown if uglify is loaded
+   * @param {function} callback a callback-function to run when the mock is active.
+   */
+  function mockRequireUglify(loadError, callback) {
+    var Module = require('module');
+    var _resolveFilename = Module._resolveFilename;
+    delete require.cache[require.resolve('uglify-js')];
+    delete require.cache[require.resolve('../dist/cjs/precompiler')];
+    Module._resolveFilename = function(request, mod) {
+      if (request === 'uglify-js') {
+        throw loadError;
+      }
+      return _resolveFilename.call(this, request, mod);
+    };
+    try {
+      callback();
+    } finally {
+      Module._resolveFilename = _resolveFilename;
+      delete require.cache[require.resolve('uglify-js')];
+      delete require.cache[require.resolve('../dist/cjs/precompiler')];
+    }
+  }
+
   beforeEach(function() {
     precompile = Handlebars.precompile;
     minify = uglify.minify;
     writeFileSync = fs.writeFileSync;
 
+    // Mock stdout and stderr
     logFunction = console.log;
     log = '';
     console.log = function() {
       log += Array.prototype.join.call(arguments, '');
     };
+    errorLogFunction = console.error;
+    errorLog = '';
+    console.error = function() {
+      errorLog += Array.prototype.join.call(arguments, '');
+    };
+
     fs.writeFileSync = function(_file, _content) {
       file = _file;
       content = _content;
@@ -46,6 +83,7 @@ describe('precompiler', function() {
     uglify.minify = minify;
     fs.writeFileSync = writeFileSync;
     console.log = logFunction;
+    console.error = errorLogFunction;
   });
 
   it('should output version', function() {
@@ -148,6 +186,29 @@ describe('precompiler', function() {
     equal(log, 'min');
   });
 
+  it('should omit minimization gracefully, if uglify-js is missing', function() {
+    var error = new Error("Cannot find module 'uglify-js'");
+    error.code = 'MODULE_NOT_FOUND';
+    mockRequireUglify(error, function() {
+      var Precompiler = require('../dist/cjs/precompiler');
+      Handlebars.precompile = function() { return 'amd'; };
+      Precompiler.cli({templates: [emptyTemplate], min: true});
+      equal(/template\(amd\)/.test(log), true);
+      equal(/\n/.test(log), true);
+      equal(/Code minimization is disabled/.test(errorLog), true);
+    });
+  });
+
+  it('should fail on errors (other than missing module) while loading uglify-js', function() {
+    mockRequireUglify(new Error('Mock Error'), function() {
+      shouldThrow(function() {
+        var Precompiler = require('../dist/cjs/precompiler');
+        Handlebars.precompile = function() { return 'amd'; };
+        Precompiler.cli({templates: [emptyTemplate], min: true});
+      }, Error, 'Mock Error');
+    });
+  });
+
   it('should output map', function() {
     Precompiler.cli({templates: [emptyTemplate], map: 'foo.js.map'});
 
