Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -907,8 +907,8 @@ res.render = function render(view, options, callback) {
opts = {};
}

// merge res.locals
opts._locals = self.locals;
// merge res.locals (into a copy, so the caller's options are not mutated)
opts = { ...opts, _locals: self.locals };

// default callback to respond
done = done || function (err, str) {
Expand Down
36 changes: 36 additions & 0 deletions test/res.render.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

var assert = require('node:assert')
var express = require('..');
var path = require('node:path')
var request = require('supertest');
Expand Down Expand Up @@ -295,6 +296,41 @@ describe('res', function(){
.get('/')
.expect('<p>jane</p>', done);
})

it('should not mutate the options object', function(done){
var app = createApp();

app.set('views', path.join(__dirname, 'fixtures'))
var options = { user: { name: 'tobi' } };

app.use(function(req, res){
res.locals.foo = 'bar';
res.render('user.tmpl', options);
});

request(app)
.get('/')
.expect('<p>tobi</p>', function (err) {
if (err) return done(err)
assert.deepStrictEqual(Object.keys(options), ['user'])
done()
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
var options = { user: { name: 'tobi' } };
app.use(function(req, res){
res.locals.foo = 'bar';
res.render('user.tmpl', options);
});
request(app)
.get('/')
.expect('<p>tobi</p>', function (err) {
if (err) return done(err)
assert.deepStrictEqual(Object.keys(options), ['user'])
done()
})
var options = { user: { name: 'tobi' } };
var optionsCopy = { ...options };
app.use(function(req, res){
res.locals.foo = 'bar';
res.render('user.tmpl', options);
});
request(app)
.get('/')
.expect(() => assert.deepStrictEqual(options, optionsCopy))
.end(done)

I think it's better to compare the objects as a whole.

})

it('should support frozen options object', function(done){
var app = createApp();

app.set('views', path.join(__dirname, 'fixtures'))
var options = Object.freeze({ user: { name: 'tobi' } });

app.use(function(req, res){
res.render('user.tmpl', options);
});

request(app)
.get('/')
.expect(200, '<p>tobi</p>', done);
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO this test is redundant

})

describe('.render(name, options, fn)', function(){
Expand Down