From 79309659e1496fead1d0a4743286398f1512b666 Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Fri, 13 Oct 2017 23:54:53 +0200 Subject: [PATCH] Gracefully handle missing uglify-js dependency 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. (cherry picked from commit d5caa56) --- lib/precompiler.js | 40 +++++++++++++++++++++++------ spec/precompiler.js | 61 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/lib/precompiler.js b/lib/precompiler.js index 6ba3800cb..64608f1e9 100644 --- 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 index 006a37e39..9f2a6442b 100644 --- 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'});