Is modifying the request object in express.js bad practice?

Question

I joined a small team of devs for a start up. We have not even launched yet. I have been handed a backend service written in node/express. I have not worked with this tech beyond small pet projects. I was looking into implementing a style guide just to keep code consistent, with the goal of implementing this across other backend services as well.

That brought me to the Airbnb style guide. This part jumped out at me. Never mutate parameters

// bad
function f1(obj) {
  obj.key = 1;
}

// good
function f2(obj) {
  const key = Object.prototype.hasOwnProperty.call(obj, 'key') ? obj.key : 1;
}

In express there are typically controllers that get defined like so:

async function someController(req, res, next) {

  // I've seen similar code to this
  req.someNewProp = "Some new value."

  res.status(200).json({"someJSONKey":"someJSONVal"});
}

Middleware typically gets defined like this:

// Route
router.get('/endpoint', function1, function2)

async function function1(req, res, next) {

  // I've seen similar code to this
  req.someNewProp = "Some new value."

  // Pass req and res to function2
  next();
}

I notice that the req object, as it gets passed around gets modified a lot. Data gets added to this object in middleware and other functions as it is passed along before the response is returned. The original dev that authored the code referred to it as "keeping things in request scope." But that seems to directly contradict a major point in the style guide and made me wonder if this is bad practice.

So the question now is, is there a "better" or more widely accepted way to keep track of things in the context of the request that is not mutating the original request object? What are some approaches of doing this?

Answer 1

Express provides a name space for applications to store request/response processing variables by adding them as properties of res.locals. This seems a better choice than attaching not standard properties to the request or response objects themselves.

In similar fashion, global application variables can be stored as properties of app.locals

Unfortunately there doesn't seem to be a locals property defined for router instances. I have placed a reference to global router instance options in res.locals as the first middleware step in a route I wrote, but that was my choice.

It can happen that request properties do need to be changed during processing, such as req.path, but this is not something to avoid at all costs. For example Express provides req.originalURL so you can recalculate path components any time you need to by deliberate design.

You may find Express gets more interesting with use - I've only recently learned of and passed an error object argument to the next function. As for the Airbnb guide quote in the post: underwhelming in a word! The "good" and "bad" code quoted in the post don't do the same thing.