I am writing a Node.js application. And there are some places where I have to modify function's arguments. For instance, this Express middleware for adding user to request so I can view it later:
exports.fetchUserDetails = function (req, res, next) {
httprequest(opts, function (err, res, body) {
req.user = body.user;
next()
}
}
The thing is, I started to use static code analyzer (ESLint) and it is always plaining about reassigning function arguments (). I guess this rule is there for a reason.
I know that modifying function parameters can be bad, like in this example:
function modifyParam(param) {
param.a = 2
}
var obj = { a: 1 };
console.log(obj); // outputs { a: 1 };
modifyParam(obj);
console.log(obj); // outputs { a: 2 };
But I don't really see the other way to refactor my middleware without arguments reassigning.
So my question is:
- When can I use params reassigning?
- How can I refactor my middleware to avoid this? (or should I leave it like it is)
I am writing a Node.js application. And there are some places where I have to modify function's arguments. For instance, this Express middleware for adding user to request so I can view it later:
exports.fetchUserDetails = function (req, res, next) {
httprequest(opts, function (err, res, body) {
req.user = body.user;
next()
}
}
The thing is, I started to use static code analyzer (ESLint) and it is always plaining about reassigning function arguments (http://eslint/docs/rules/no-param-reassign). I guess this rule is there for a reason.
I know that modifying function parameters can be bad, like in this example:
function modifyParam(param) {
param.a = 2
}
var obj = { a: 1 };
console.log(obj); // outputs { a: 1 };
modifyParam(obj);
console.log(obj); // outputs { a: 2 };
But I don't really see the other way to refactor my middleware without arguments reassigning.
So my question is:
- When can I use params reassigning?
- How can I refactor my middleware to avoid this? (or should I leave it like it is)
- Why do you consider the example bad? – abukaj Commented Apr 26, 2019 at 16:50
2 Answers
Reset to default 5I think in this case it's fine. You're setting up state that'll be used by subsequent functions that process the request.
The reason linters plain about this, is that it's often unclear when calling a function, that it will modify its arguments, leading to bugs, as you described in your question.
But in this case, your function has only one caller, the express framework, and it's always clear under which circumstances your function will be called, so I don't think it's a problem here.
Example you provided does not include reassigning function arguments.
exports.fetchUserDetails = function (req, res, next) {
httprequest(opts, function (err, res, body) {
req.user = body.user;
next()
}
}
You just attach new field to req
reference, but you do not overriding req
itself.
Express middleware is using this approach from the beginning, and there is nothing wrong with that.