[Enesuygurs]

res.render(view, options) merges res.locals by assigning it to options._locals, directly on the object passed by the caller. This has two consequences:

• Passing a frozen (or otherwise non-extensible) options object fails the request with TypeError: Cannot add property _locals, object is not extensible.
• Reusing an options object across requests (e.g. a module-level constant) leaves the previous request's res.locals attached to it as _locals. That keeps the data alive and exposes it if the object is later inspected or serialized.

const options = Object.freeze({ title: 'Home' })

app.get('/', (req, res) => {
  res.render('index', options) // 500: Cannot add property _locals, object is not extensible
})

This change merges res.locals into a shallow copy of the options instead. What view engines receive is unchanged, since app.render() already builds a new renderOptions object from app.locals, options._locals and options (own enumerable properties only).

I added tests for both cases; they fail without the change. npm test passes with 1263 tests, compared to 1261 on master, and npm run lint is clean.

The same assignment exists on 4.x (lib/response.js). I'm happy to open a backport there if wanted.

[krzysdz]

We definitely should not modify the options arguments. I found a PR from 2013 that removed one such mutation (#1458, utils-merge was later replaced by spread in #6091), but there are still similar patterns in our code.

Another case where a function argument is mutated is here:

express/lib/response.js

Line 404 in 0dee2c5

opts.etag = this.app.enabled('etag');

This particular line is slightly wrong also for another reason, but #7467 deals with it, so I left a comment there.

View also modifies one of its arguments, but it is an expected behaviour and it is an internal function, so no changes are necessary.

[krzysdz]

IMO this test is redundant

[krzysdz]

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.

[Enesuygurs]

Applied both suggestions: removed the frozen-options test, and the remaining test now compares the whole options object against a copy. Thanks for the review!