Skip to content

Commit

Permalink
Fix localsAsTemplateData when caching is enabled
Browse files Browse the repository at this point in the history
When a template is rendered from the cache, the extra options object
(containing the data property) isn't being passed in to the render
function. Thus, any functionality that depends on this will fail.

The code has been refactored to create a single handlebars options
object for each (main) template render, which is then passed in all
scenarios (cache, template, layout template).

The code also cleans up the use of parent-scope variables for the render
context, and avoids a circular reference to the "_locals" object from
itself in favor of an instance-based boolean to determine whether the
locals should be passed as template data.

Tests have also been added to test for this scenario.

fixes #101
  • Loading branch information
dpolivy committed Jul 24, 2015
1 parent b256cdf commit 720395b
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 13 deletions.
23 changes: 10 additions & 13 deletions lib/hbs.js
Expand Up @@ -37,29 +37,30 @@ function middleware(filename, options, cb) {
// if we need a layout, we will look for one matching out extension
var extension = path.extname(filename);

// If passing the locals as data, create the handlebars options object now
var handlebarsOpts = (self.__localsAsData) ? { data: options._locals } : undefined;

// render the original file
// cb(err, str)
function render_file(locals, cb) {
// cached?
var template = cache[filename];
if (template) {
return cb(null, template(locals));
return cb(null, template(locals, handlebarsOpts));
}

fs.readFile(filename, 'utf8', function(err, str){
if (err) {
return cb(err);
}

var locals = options;
var template = handlebars.compile(str);
if (options.cache) {
if (locals.cache) {
cache[filename] = template;
}

try {
var data = locals.__hbsLocals;
var res = template(locals, { data: data });
var res = template(locals, handlebarsOpts);
self.async.done(function(values) {
Object.keys(values).forEach(function(id) {
res = res.replace(id, values[id]);
Expand All @@ -81,12 +82,9 @@ function middleware(filename, options, cb) {
return cb(err);
}

var locals = options;
locals.body = str;

var data = locals.__hbsLocals;
delete locals.__hbsLocals;
var res = template(locals, { data: data });
var res = template(locals, handlebarsOpts);
self.async.done(function(values) {
Object.keys(values).forEach(function(id) {
res = res.replace(id, values[id]);
Expand Down Expand Up @@ -237,6 +235,9 @@ Instance.prototype.registerAsyncHelper = function(name, fn) {
};

Instance.prototype.localsAsTemplateData = function(app) {
// Set a flag to indicate we should pass locals as data
this.__localsAsData = true;

app.render = (function(render) {
return function(view, options, callback) {
if (typeof options === "function") {
Expand All @@ -250,10 +251,6 @@ Instance.prototype.localsAsTemplateData = function(app) {
options._locals[key] = this.locals[key];
}

// Store the data again, so that we can differentiate this data from
// the data passed to response.data() when we're inside the view
options._locals.__hbsLocals = options._locals;

return render.call(this, view, options, callback);
};
})(app.render);
Expand Down
30 changes: 30 additions & 0 deletions test/3.x/app.js
Expand Up @@ -140,6 +140,15 @@ app.get('/locals', function(req, res) {
});
});

app.get('/locals-cached', function(req, res) {
res.locals.person = 'Alan';
res.render('locals', {
layout: false,
cache: true,
kids: [{ name: 'Jimmy' }, { name: 'Sally' }],
});
});

app.get('/globals', function(req, res) {
res.render('globals', {
layout: 'layout_globals',
Expand Down Expand Up @@ -242,6 +251,27 @@ test('response locals', function(done) {
});
});

test('response locals cached', function(done) {
var server = app.listen(3000, function() {

var expected = fs.readFileSync(__dirname + '/../fixtures/locals.html', 'utf8');

request('http://localhost:3000/locals-cached', function(err, res, body) {
assert.equal(body, expected);

// Request the second time, so it is cached
request('http://localhost:3000/locals-cached', function(err, res, body) {
assert.equal(body, expected);
server.close();
});
});
});

server.on('close', function() {
done();
});
});

test('response globals', function(done) {
var server = app.listen(3000, function() {

Expand Down
30 changes: 30 additions & 0 deletions test/4.x/app.js
Expand Up @@ -136,6 +136,15 @@ app.get('/locals', function(req, res) {
});
});

app.get('/locals-cached', function(req, res) {
res.locals.person = 'Alan';
res.render('locals', {
layout: false,
cache: true,
kids: [{ name: 'Jimmy' }, { name: 'Sally' }],
});
});

app.get('/globals', function(req, res) {
res.render('globals', {
layout: 'layout_globals',
Expand Down Expand Up @@ -250,6 +259,27 @@ test('response locals', function(done) {
});
});

test('response locals cached', function(done) {
var server = app.listen(3000, function() {

var expected = fs.readFileSync(__dirname + '/../fixtures/locals.html', 'utf8');

request('http://localhost:3000/locals-cached', function(err, res, body) {
assert.equal(body, expected);

// Request the second time, so it is cached
request('http://localhost:3000/locals-cached', function(err, res, body) {
assert.equal(body, expected);
server.close();
});
});
});

server.on('close', function() {
done();
});
});

test('response globals', function(done) {
var server = app.listen(3000, function() {

Expand Down

0 comments on commit 720395b

Please sign in to comment.