Skip to content

Commit

Permalink
Gracefully handle missing uglify-js dependency
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
nknapp committed Oct 17, 2017
1 parent 5b76f04 commit 7930965
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 8 deletions.
40 changes: 32 additions & 8 deletions lib/precompiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -235,7 +235,6 @@ module.exports.cli = function(opts) {
}
}


if (opts.map) {
output.add('\n//# sourceMappingURL=' + opts.map + '\n');
}
Expand All @@ -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) {
Expand All @@ -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)
});
}
61 changes: 61 additions & 0 deletions spec/precompiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ describe('precompiler', function() {

var log,
logFunction,
errorLog,
errorLogFunction,

precompile,
minify,
Expand All @@ -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;
Expand All @@ -46,6 +83,7 @@ describe('precompiler', function() {
uglify.minify = minify;
fs.writeFileSync = writeFileSync;
console.log = logFunction;
console.error = errorLogFunction;
});

it('should output version', function() {
Expand Down Expand Up @@ -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'});

Expand Down

0 comments on commit 7930965

Please sign in to comment.