Skip to content

Commit

Permalink
fixes for engines, helpers and context:
Browse files Browse the repository at this point in the history 
**Context**

- in general, context should be merged so that the most specific context wins over less specific. This fixes one case where locals was winning over front-matter

**Helpers**

- removes orphan params on `bindHelpers`
- exposes `.ctx()` method on helper context, to simplify merging context in non-built-in helpers

**Engines**

- fixes bug that was using default engine on options instead of engine that matches view file extension.
  • Loading branch information
@jonschlinkert
jonschlinkert committed May 26, 2016
1 parent 8b4c8fe commit e54e448
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 94 deletions.
43 changes: 3 additions & 40 deletions lib/helpers/singular.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ module.exports = function(app, collection) {
return;
}

var ctx = helperContext.call(this, view, locals, opts);
debug('"%s" pre-rendering "%s"', single, name);

view.render(ctx, function(err, res) {
view._context = this.ctx(view, locals, opts);
view.render(function(err, res) {
if (err) return cb(err);

debug('"%s" rendered "%s"', single, name);
Expand All @@ -71,44 +71,7 @@ module.exports = function(app, collection) {

function helperOptions(locals, options) {
var hash = options.hash || locals.hash || {};
var opts = utils.extend({}, this.options, hash);
var opts = utils.merge({}, this.options, hash);
opts.hash = hash;
return opts;
}

/**
* Create the context to use for rendering from:
*
* - helper `locals`
* - helper `options`
* - context (`this.context`)
* - `options.hash` if it's registered as a handlebars helper
* - `view.locals`: locals defined on the view being rendered
* - `view.data`: data from front-matter
*
* @param {Object} `view` the view being rendered
* @param {Object} `locals` Helper locals
* @param {Object} `options` Helper options
* @return {Object}
*/

function helperContext(view, locals, options) {
var fn = this.options.helperContext;
var extend = utils.extend;
var context = {};

if (typeof fn === 'function') {
context = fn.call(this, view, locals, options);
} else {
// helper context
context = extend({}, this.view.data);
context = extend({}, context, this.context);

// view context
context = extend({}, context, view.locals, view.data);

// helper locals and options
context = extend({}, context, locals, options.hash);
}
return context;
}
122 changes: 83 additions & 39 deletions lib/plugins/context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ module.exports = function(app) {
* Bind context to helpers.
*/

app.define('bindHelpers', function(view, locals, context, isAsync) {
app.define('bindHelpers', function(view, context, isAsync) {
debug.context('binding helpers for %s <%s>', view.options.inflection, view.basename);
var optsHelpers = utils.isObject(this.options.helpers) ? this.options.helpers : {};

Expand All @@ -88,8 +88,8 @@ module.exports = function(app) {
helpers = utils.extend({}, helpers, this._.helpers.async);
}

// create the context to expose as `this` inside helper functions
var thisArg = new Context(this, view, locals, context, helpers);
// create the "helper context" to be exposed as `this` inside helper functions
var thisArg = new Context(this, view, context, helpers);

// bind the context to helpers.
context.helpers = utils.bindAll(helpers, thisArg, {
Expand Down Expand Up @@ -129,17 +129,17 @@ module.exports = function(app) {
* // 'this.app' => the application instance, e.g. templates, assemble, verb etc.
* // 'this.view' => the current view being rendered
* // 'this.helper' => helper name and options
* // 'this.context' => template context (as opposed to _helper_ context)
* // 'this.options' => merged options from app, view, and helper options
* // 'this.context' => view context (as opposed to _helper_ context)
* // 'this.options' => options created for the specified helper being called
* });
* ```
* @param {Object} `app`
* @param {Object} `view`
* @param {Object} `context`
* @param {Object} `app` The application instance
* @param {Object} `view` The view being rendered
* @param {Object} `context` The view's context
* @param {Object} `options`
*/

function Context(app, view, locals, context, helpers) {
function Context(app, view, context, helpers) {
this.helper = {};
this.helper.options = createHelperOptions(app, view, helpers);

Expand All @@ -155,6 +155,72 @@ module.exports = function(app) {

this.view = view;
this.app = app;
this.ctx = function() {
return helperContext.apply(this, arguments);
};
}

/**
* Expose `this.ctx()` in helpers for creating a custom context object
* from a `view` being rendered, `locals` and helper `options` (which
* can optionally have a handlebars `options.hash` property)
*
* The context object is created from:
*
* - `this.context`: the context for the **current view being rendered**
* - `this.view.locals`: the `locals` object for current view being rendered
* - `this.view.data`: front-matter from current view being rendered
* - `view.locals`: locals defined on the view being injected
* - `view.data`: front-matter on the view being injected
* - helper `locals`: locals passed to the helper
* - helper `options`: options passed to the helper
* - helper `options.hash` if helper is registered as a handlebars helper
*
* Also note that `view` is the view being injected, whereas `this.view`
* is the `view` being rendered.
*
* ```js
* // handlebars helper
* app.helper('foo', function(name, locals, options) {
* var view = this.app.find(name);
* var ctx = this.ctx(view, locals, options);
* return options.fn(ctx);
* });
*
* // async helper
* app.helper('foo', function(name, locals, options, cb) {
* var view = this.app.find(name);
* var ctx = this.ctx(view, locals, options);
* view.render(ctx, function(err, res) {
* if (err) return cb(err);
* cb(null, res.content);
* });
* });
* ```
* @name .this.ctx
* @param {Object} `view` the view being injected
* @param {Object} `locals` Helper locals
* @param {Object} `options` Helper options
* @return {Object}
* @public
*/

function helperContext(view, locals, options) {
var fn = this.options.helperContext;
var merge = utils.merge;
var context = {};

if (typeof fn === 'function') {
context = fn.call(this, view, locals, options);
} else {
// merge "view" front-matter with context
context = merge({}, this.context, this.view.data);
// merge in partial locals and front-matter
context = merge({}, context, view.locals, view.data);
// merge in helper locals and options.hash
context = merge({}, context, locals, options.hash);
}
return context;
}

/**
Expand Down Expand Up @@ -212,7 +278,9 @@ module.exports = function(app) {

if (options.hasOwnProperty('helper')) {
var opts = options.helper;
if (!utils.isObject(opts)) return helperOptions;
if (!utils.isObject(opts)) {
return helperOptions;
}

for (var key in opts) {
if (opts.hasOwnProperty(key) && helpers.hasOwnProperty(key)) {
Expand All @@ -234,8 +302,6 @@ module.exports = function(app) {
* @api public
*/

app.define('mergePartials', mergePartials);

function mergePartials(options) {
var opts = utils.merge({}, this.options, options);
var names = opts.mergeTypes || this.viewTypes.partial;
Expand Down Expand Up @@ -315,34 +381,12 @@ module.exports = function(app) {
if (err) return done(err);
done(null, partials);
});

// async.reduce(names, partials, function(acc, name, cb) {
// var collection = self.views[name];
// async.eachOf(collection, function(view, key, next) {
// // handle `onMerge` middleware
// self.handleOnce('onMerge', view, function(err, file) {
// if (err) return next(err);

// if (file.options.nomerge) {
// return next();
// }

// if (opts.mergePartials !== false) {
// name = 'partials';
// }

// // convert the partial to:
// //=> {'foo.hbs': 'some content...'};
// acc[name] = acc[name] || {};
// acc[name][key] = file.content;
// next(null, acc);
// });
// }, function(err) {
// if (err) return cb(err);
// cb(null, partials);
// });
// }, done);
};

/**
* Expose `mergePartials` functions as methods
*/

app.define('mergePartials', mergePartials);
app.define('mergePartialsAsync', mergePartials.async);
};
26 changes: 20 additions & 6 deletions lib/plugins/engine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,34 +49,48 @@ module.exports = function(proto) {
};

/**
* Register an engine for `ext` with the given `settings`
* Register engine `ext` with the given render `fn` and/or `settings`.
*
* @param {String} `ext` The engine to get.
* ```js
* app.setEngine('hbs', require('engine-handlebars'), {
* delims: ['<%', '%>']
* });
* ```
* @param {String} `ext` The engine to set.
*/

proto.setEngine = function(ext, fn, settings) {
ext = utils.formatExt(ext);
debug('registering engine "%s"', ext);
ext = utils.formatExt(ext);
settings = settings || {};
if (settings.default === true) {
this._.engines.defaultEngine = ext;
}
this._.engines.setEngine(ext, fn, settings);
return this;
};

/**
* Get the engine settings registered for the given `ext`.
* Get registered engine `ext`.
*
* ```js
* app.engine('hbs', require('engine-handlebars'));
* var engine = app.getEngine('hbs');
* ```
* @param {String} `ext` The engine to get.
* @api public
*/

proto.getEngine = function(ext) {
debug('getting engine "%s"', ext);

if (typeof ext !== 'string') {
if (!utils.isString(ext)) {
ext = this.option('view engine')
|| this.option('viewEngine')
|| this.option('engine');
}

if (typeof ext === 'string') {
if (utils.isString(ext)) {
ext = utils.formatExt(ext);
return this._.engines.getEngine(ext);
}
Expand Down
5 changes: 3 additions & 2 deletions lib/plugins/render.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ module.exports = function(proto) {
view = this.applyLayout(view);

// Bind context to helpers before passing to the engine.
this.bindHelpers(view, locals, ctx, (ctx.async = !!isAsync));
this.bindHelpers(view, ctx, (ctx.async = !!isAsync));

// shallow clone the context and locals
engineOpts = utils.merge({}, ctx, engineOpts, this.mergePartials(engineOpts));
Expand Down Expand Up @@ -213,7 +213,7 @@ module.exports = function(proto) {
if (err) return cb(err);

// Bind context to helpers before passing to the engine.
app.bindHelpers(view, locals, ctx, (ctx.async = !!isAsync));
app.bindHelpers(view, ctx, (ctx.async = !!isAsync));

app.mergePartialsAsync(engineOpts, function(err, partials) {
if (err) return cb(err);
Expand Down Expand Up @@ -309,6 +309,7 @@ module.exports = function(proto) {

view.localsStack.push(locals);
view.content = res;
delete view._context;

// handle `postRender` middleware
app.handleOnce('postRender', view, cb);
Expand Down
3 changes: 2 additions & 1 deletion lib/plugins/views.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ module.exports = function(app, views, options) {
views.setType(view);
// bind the `addView` method to allow chaining
utils.define(view, 'addView', views.addView.bind(views));

// pass the engine defined on `collection.options` to `view.options`
view.engine = views.options.engine || view.engine;
if (!view.engine) view.engine = views.options.engine;
app.extendView(view, options);
});

Expand Down
8 changes: 8 additions & 0 deletions lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,14 @@ utils.isObject = function(val) {
|| typeof val === 'object';
};

/**
* Return true if the given value is a string.
*/

utils.isString = function(val) {
return val && typeof val === 'string';
};

/**
* Return true if the given value is a stream.
*/
Expand Down
15 changes: 11 additions & 4 deletions lib/view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,20 @@ Item.extend(View);
*/

View.prototype.context = function(locals) {
if (this._context) return this._context;

if (typeof locals === 'function') {
return locals.call(this, this);
}

if (arguments.length > 1) {
locals = [].concat.apply([], [].slice.call(arguments));
} else {
locals = utils.arrayify(locals);
}
locals.unshift(utils.merge({}, this.locals, this.data));

locals.unshift(this.locals);
locals.push(this.data);
return utils.merge.apply(utils.merge, locals);
};

Expand Down Expand Up @@ -112,6 +120,7 @@ View.prototype.render = function(locals, cb) {
utils.engine.render(this.fn, context, function(err, res) {
if (err) return cb(err);
this.contents = new Buffer(res);
delete this._context;
cb(null, this);
}.bind(this));
return this;
Expand Down Expand Up @@ -224,7 +233,5 @@ function resolveEngine(view) {
engine = path.extname(view.path);
view.data.ext = engine;
}
if (engine) {
return engine;
}
return engine;
}
4 changes: 2 additions & 2 deletions test/app.render.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ describe('app.render', function() {
});
});

it('should use passed in locals in template.', function(cb) {
it('should use front-matter over locals passed on render', function(cb) {
app.data({name: 'CCC'});
app.page('a.tmpl', {content: 'a <%= name %> b', data: {name: 'DDD'}});
app.render('a.tmpl', {name: 'EEE'}, function(err, res) {
if (err) return cb(err);
res.content.should.equal('a EEE b');
res.content.should.equal('a DDD b');
cb();
});
});
Expand Down

0 comments on commit e54e448

Please sign in to comment.