Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat - #175 adds processSource option to the loader config to run han… #177

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
@@ -1,5 +1,6 @@
*.sublime-project
*.sublime-workspace
*.code-workspace
node_modules
example/dist/
.ntvs_analysis.dat
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,11 @@
### Fixed
- Your improvement here...

## [1.7.2] - 2019-01-09

### Fixed
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### Fixed
### New Features

- Added `processSource` option to run handlebars template through html minifier for production optimizations (#175)

## [1.7.1] - 2018-12-18

### Fixed
Expand Down
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -64,6 +64,7 @@ The following query (or config) options are supported:
- *ignorePartials*: Prevents partial references from being fetched and bundled. Useful for manually loading partials at runtime.
- *ignoreHelpers*: Prevents helper references from being fetched and bundled. Useful for manually loading helpers at runtime.
- *precompileOptions*: Options passed to handlebars precompile. See the Handlebars.js [documentation](http://handlebarsjs.com/reference.html#base-precompile) for more information.
- *processSource*: Runs the handlebars templates through an html minifier to remove whitespace and reduce the size of the compiled templates.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

processSource is a very generic name that makes it hard to guess how exactly the source is being processed. Maybe something like minifyOutput would be better?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think minifyOutput is a much clearer name. Will make the change.

- *config*: Tells the loader where to look in the webpack config for configurations for this loader. Defaults to `handlebarsLoader`.
- *config.partialResolver* You can specify a function to use for resolving partials. To do so, add to your webpack config:
```js
Expand Down
103 changes: 59 additions & 44 deletions index.js
Expand Up @@ -6,6 +6,7 @@ var path = require("path");
var assign = require("object-assign");
var fastreplace = require('./lib/fastreplace');
var findNestedRequires = require('./lib/findNestedRequires');
var minify = require('html-minifier').minify;
Copy link
Author

@pizatski pizatski Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is an added dependency. I feel like it's probably better to add a known and tested library to do this sort of thing than writing my own regular expressions which will surely be buggy. Kudos to my colleague Clinton Bloodworth for the suggestion!


function versionCheck(hbCompiler, hbRuntime) {
return hbCompiler.COMPILER_REVISION === (hbRuntime["default"] || hbRuntime).COMPILER_REVISION;
Expand All @@ -26,7 +27,23 @@ function getLoaderConfig(loaderContext) {
return assign({}, config, query);
}

module.exports = function(source) {
/**
* Returns the template source with newlines and multiple spaces removed
* based on the processSource config option.
*
* @param {String} source
* @returns {String}
*/
var processSource = function (source) {
return minify(source, {
collapseWhitespace: true,
conservativeCollapse: true,
removeComments: true,
removeAttributeQuotes: true
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are fairly conservative options. More can be found here:
https://www.npmjs.com/package/html-minifier

});
};

module.exports = function (source) {
if (this.cacheable) this.cacheable();
var loaderApi = this;
var query = getLoaderConfig(loaderApi);
Expand All @@ -42,8 +59,7 @@ module.exports = function(source) {
var extensions = query.extensions;
if (!extensions) {
extensions = [".handlebars", ".hbs", ""];
}
else if (!Array.isArray(extensions)) {
} else if (!Array.isArray(extensions)) {
extensions = extensions.split(/[ ,;]/g);
}

Expand All @@ -57,7 +73,7 @@ module.exports = function(source) {
var foundUnclearStuff = {};
var knownHelpers = {};

[].concat(query.knownHelpers, precompileOptions.knownHelpers).forEach(function(k) {
[].concat(query.knownHelpers, precompileOptions.knownHelpers).forEach(function (k) {
if (k && typeof k === 'string') {
knownHelpers[k] = true;
}
Expand All @@ -77,12 +93,13 @@ module.exports = function(source) {

var hb = handlebars.create();
var JavaScriptCompiler = hb.JavaScriptCompiler;

function MyJavaScriptCompiler() {
JavaScriptCompiler.apply(this, arguments);
}
MyJavaScriptCompiler.prototype = Object.create(JavaScriptCompiler.prototype);
MyJavaScriptCompiler.prototype.compiler = MyJavaScriptCompiler;
MyJavaScriptCompiler.prototype.nameLookup = function(parent, name, type) {
MyJavaScriptCompiler.prototype.nameLookup = function (parent, name, type) {
if (debug) {
console.log("nameLookup %s %s %s", parent, name, type);
}
Expand All @@ -96,26 +113,23 @@ module.exports = function(source) {
}
foundPartials["$" + name] = null;
return JavaScriptCompiler.prototype.nameLookup.apply(this, arguments);
}
else if (type === "helper") {
} else if (type === "helper") {
if (foundHelpers["$" + name]) {
return "__default(require(" + loaderUtils.stringifyRequest(loaderApi, foundHelpers["$" + name]) + "))";
}
foundHelpers["$" + name] = null;
return JavaScriptCompiler.prototype.nameLookup.apply(this, arguments);
}
else if (type === "context") {
} else if (type === "context") {
// This could be a helper too, save it to check it later
if (!foundUnclearStuff["$" + name]) foundUnclearStuff["$" + name] = false;
return JavaScriptCompiler.prototype.nameLookup.apply(this, arguments);
}
else {
} else {
return JavaScriptCompiler.prototype.nameLookup.apply(this, arguments);
}
};

if (inlineRequires) {
MyJavaScriptCompiler.prototype.pushString = function(value) {
MyJavaScriptCompiler.prototype.pushString = function (value) {
if (inlineRequires.test(value)) {
this.pushLiteral("require(" + loaderUtils.stringifyRequest(loaderApi, value) + ")");
} else {
Expand All @@ -138,17 +152,18 @@ module.exports = function(source) {

// Define custom visitor for further template AST parsing
var Visitor = handlebars.Visitor;

function InternalBlocksVisitor() {
this.partialBlocks = [];
this.inlineBlocks = [];
}

InternalBlocksVisitor.prototype = new Visitor();
InternalBlocksVisitor.prototype.PartialBlockStatement = function(partial) {
InternalBlocksVisitor.prototype.PartialBlockStatement = function (partial) {
this.partialBlocks.push(partial.name.original);
Visitor.prototype.PartialBlockStatement.call(this, partial);
};
InternalBlocksVisitor.prototype.DecoratorBlock = function(partial) {
InternalBlocksVisitor.prototype.DecoratorBlock = function (partial) {
if (partial.path.original === 'inline') {
this.inlineBlocks.push(partial.params[0].value);
}
Expand Down Expand Up @@ -202,14 +217,17 @@ module.exports = function(source) {

try {
if (source) {
if (query.processSource) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got a lint error when I double banged this. Let me know if that's okay.

source = processSource(source);
}
ast = hb.parse(source, opts);
template = hb.precompile(ast, opts);
}
} catch (err) {
return loaderAsyncCallback(err);
}

var resolve = function(request, type, callback) {
var resolve = function (request, type, callback) {
var contexts = [loaderApi.context];

// Any additional helper dirs will be added to the searchable contexts
Expand All @@ -222,7 +240,7 @@ module.exports = function(source) {
contexts = contexts.concat(query.partialDirs);
}

var resolveWithContexts = function() {
var resolveWithContexts = function () {
var context = contexts.shift();

var traceMsg;
Expand All @@ -232,28 +250,25 @@ module.exports = function(source) {
console.log("request=%s", request);
}

var next = function(err) {
var next = function (err) {
if (contexts.length > 0) {
resolveWithContexts();
}
else {
} else {
if (debug) console.log("Failed to resolve %s %s", type, traceMsg);
return callback(err);
}
};

loaderApi.resolve(context, request, function(err, result) {
loaderApi.resolve(context, request, function (err, result) {
if (!err && result) {
if (exclude && exclude.test(result)) {
if (debug) console.log("Excluding %s %s", type, traceMsg);
return next();
}
else {
} else {
if (debug) console.log("Resolved %s %s", type, traceMsg);
return callback(err, result);
}
}
else {
} else {
return next(err);
}
});
Expand All @@ -262,14 +277,14 @@ module.exports = function(source) {
resolveWithContexts();
};

var resolveUnclearStuffIterator = function(stuff, unclearStuffCallback) {
var resolveUnclearStuffIterator = function (stuff, unclearStuffCallback) {
if (foundUnclearStuff[stuff]) return unclearStuffCallback();
var request = referenceToRequest(stuff.substr(1), 'unclearStuff');

if (query.ignoreHelpers) {
unclearStuffCallback();
} else {
resolve(request, 'unclearStuff', function(err, result) {
resolve(request, 'unclearStuff', function (err, result) {
if (!err && result) {
knownHelpers[stuff.substr(1)] = true;
foundHelpers[stuff] = result;
Expand All @@ -281,7 +296,7 @@ module.exports = function(source) {
}
};

var defaultPartialResolver = function defaultPartialResolver(request, callback){
var defaultPartialResolver = function defaultPartialResolver(request, callback) {
request = referenceToRequest(request, 'partial');
// Try every extension for partials
var i = 0;
Expand All @@ -292,7 +307,7 @@ module.exports = function(source) {
}
var extension = extensions[i++];

resolve(request + extension, 'partial', function(err, result) {
resolve(request + extension, 'partial', function (err, result) {
if (!err && result) {
return callback(null, result);
}
Expand All @@ -301,18 +316,18 @@ module.exports = function(source) {
}());
};

var resolvePartialsIterator = function(partial, partialCallback) {
var resolvePartialsIterator = function (partial, partialCallback) {
if (foundPartials[partial]) return partialCallback();
// Strip the # off of the partial name
var request = partial.substr(1);

var partialResolver = query.partialResolver || defaultPartialResolver;

if(query.ignorePartials) {
if (query.ignorePartials) {
return partialCallback();
} else {
partialResolver(request, function(err, resolved){
if(err) {
partialResolver(request, function (err, resolved) {
if (err) {
var visitor = new InternalBlocksVisitor();

visitor.accept(ast);
Expand All @@ -334,20 +349,20 @@ module.exports = function(source) {
}
};

var resolveHelpersIterator = function(helper, helperCallback) {
var resolveHelpersIterator = function (helper, helperCallback) {
if (foundHelpers[helper]) return helperCallback();
var request = referenceToRequest(helper.substr(1), 'helper');

if (query.ignoreHelpers) {
helperCallback();
} else {
var defaultHelperResolver = function(request, callback){
var defaultHelperResolver = function (request, callback) {
return resolve(request, 'helper', callback);
};

var helperResolver = query.helperResolver || defaultHelperResolver;

helperResolver(request, function(err, result) {
helperResolver(request, function (err, result) {
if (!err && result) {
knownHelpers[helper.substr(1)] = true;
foundHelpers[helper] = result;
Expand All @@ -363,7 +378,7 @@ module.exports = function(source) {
}
};

var doneResolving = function(err) {
var doneResolving = function (err) {
if (err) return loaderAsyncCallback(err);

// Do another compiler pass if not everything was resolved
Expand All @@ -374,15 +389,15 @@ module.exports = function(source) {

// export as module if template is not blank
var slug = template ?
'var Handlebars = require(' + loaderUtils.stringifyRequest(loaderApi, runtimePath) + ');\n'
+ 'function __default(obj) { return obj && (obj.__esModule ? obj["default"] : obj); }\n'
+ 'module.exports = (Handlebars["default"] || Handlebars).template(' + template + ');' :
'var Handlebars = require(' + loaderUtils.stringifyRequest(loaderApi, runtimePath) + ');\n' +
'function __default(obj) { return obj && (obj.__esModule ? obj["default"] : obj); }\n' +
'module.exports = (Handlebars["default"] || Handlebars).template(' + template + ');' :
'module.exports = function(){return "";};';

loaderAsyncCallback(null, slug);
};

var resolveItems = function(err, type, items, iterator, callback) {
var resolveItems = function (err, type, items, iterator, callback) {
if (err) return callback(err);

var itemKeys = Object.keys(items);
Expand All @@ -395,18 +410,18 @@ module.exports = function(source) {
async.each(itemKeys, iterator, callback);
};

var resolvePartials = function(err) {
var resolvePartials = function (err) {
resolveItems(err, 'partials', foundPartials, resolvePartialsIterator, doneResolving);
};

var resolveUnclearStuff = function(err) {
var resolveUnclearStuff = function (err) {
resolveItems(err, 'unclearStuff', foundUnclearStuff, resolveUnclearStuffIterator, resolvePartials);
};

var resolveHelpers = function(err) {
var resolveHelpers = function (err) {
resolveItems(err, 'helpers', foundHelpers, resolveHelpersIterator, resolveUnclearStuff);
};

resolveHelpers();
}());
};
};
6 changes: 4 additions & 2 deletions package.json
Expand Up @@ -3,7 +3,8 @@
"contributors": [
"Alan @altano",
"Tobias Koppers @sokra",
"Jason Anderson @diurnalist"
"Jason Anderson @diurnalist",
"Patrick Szczypinski @pizatski"
],
"maintainers": [
"Paul Carduner @pcardune"
Expand All @@ -12,6 +13,7 @@
"dependencies": {
"async": "~0.2.10",
"fastparse": "^1.0.0",
"html-minifier": "^3.5.21",
"loader-utils": "1.0.x",
"object-assign": "^4.1.0"
},
Expand All @@ -28,7 +30,7 @@
"type": "git",
"url": "git://github.com/pcardune/handlebars-loader.git"
},
"version": "1.7.1",
"version": "1.7.2",
"devDependencies": {
"coveralls": "^2.11.8",
"eslint": "^2.8.0",
Expand Down
18 changes: 17 additions & 1 deletion test/test.js
Expand Up @@ -556,4 +556,20 @@ describe('handlebars-loader', function () {
});
});

});
/*
* This uses an html-minifier with 'conservative whitespace stripping' on. This means you'll still get a single space around each
* var in the handlebars template which can result in two spaces between tags.
*/
it('should output with fewer than three spaces and no newlines when processSource option is set', function (done) {
testTemplate(loader, './simple.handlebars', {
query: '?processSource=true',
data: TEST_TEMPLATE_DATA
}, function (err, output, require) {
assert.ok(output, 'generated output');
assert.ok(output.indexOf('\n') === -1);
assert.ok(output.indexOf(' ') === -1);
done();
});
});

});