162

I have a method which's main purpose is to set a property on a DOM object

function (el) {
  el.expando = {};
}

I use AirBnB's code style which makes ESLint throw a no-param-reassign error:

error Assignment to function parameter 'el' no-param-reassign

How can I manipulate a DOM object passed as an argument while conforming AirBnB's code style?

Somebody suggested to use /* eslint react/prop-types: 0 */ referring to another issue but if I am not mistaken this applies well for react, but not for native DOM manipulation.

Also I do not think changing the code style is an answer. I believe one of the benefits of using a standard style is having consistent code across projects and changing the rules at will feels like a misuse of a major code style like AirBnB's.

For the record, I asked AirBnB on GitHub, what they think is the way to go in these cases in issue #766.

Community
  • 1
  • 1
Lukas
  • 9,752
  • 15
  • 76
  • 120
  • Nah. Firstly, that would mean disabling this for all other occurrences where this rule makes sense. Secondly I believe you either follow a style guide or don't. At least if it is a style guide followed by many developers across all kinds of projects. – Lukas Feb 25 '16 at 20:48
  • 3
    But you are asking how _not_ to obey the styleguide, because you are doing the the thing it is trying to prevent. In any case, just [disable it for that function](http://eslint.org/docs/user-guide/configuring#configuring-rules) – Evan Davis Feb 25 '16 at 20:51
  • Possible duplicate of [How to disable ESLint react/prop-types rule in a fil?](http://stackoverflow.com/questions/30948970/how-to-disable-eslint-react-prop-types-rule-in-a-fil) – Evan Davis Feb 25 '16 at 20:52
  • @Mathletics No I find the rule sensical, but it just does not work for this specific case. I was wondering if there is a way to do this playing along the rules. – Lukas Feb 25 '16 at 20:54
  • No matter how you word it, the operation you want conflicts with the rule. That said, it seems like an XY problem; I wouldn't attach properties directly to DOM nodes like that. – Evan Davis Feb 25 '16 at 20:59
  • 1
    @Lukas "Secondly I believe you either follow a style guide or don't". Only when it makes sense. Blindly following something is cargo-cult programming. – Hejazzman Oct 26 '16 at 12:45

13 Answers13

164

As this article explains, this rule is meant to avoid mutating the arguments object. If you assign to a parameter and then try and access some of the parameters via the arguments object, it can lead to unexpected results.

You could keep the rule intact and maintain the AirBnB style by using another variable to get a reference to the DOM element and then modify that:

function (el) {
  var theElement = el;
  theElement.expando = {};
}

In JS objects (including DOM nodes) are passed by reference, so here el and theElement are references to the same DOM node, but modifying theElement doesn't mutate the arguments object since arguments[0] remains just a reference to that DOM element.

This approach is hinted at in the documentation for the rule:

Examples of correct code for this rule:

/*eslint no-param-reassign: "error"*/

function foo(bar) {
    var baz = bar;
}

Personally, I would just use the "no-param-reassign": ["error", { "props": false }] approach a couple of other answers mentioned. Modifying a property of an object that was passed as a parameter doesn't change the object reference, so it shouldn't run into the kinds of problems this rule is trying to avoid.

Useless Code
  • 12,123
  • 5
  • 35
  • 40
  • 13
    This should be the accepted answer, not a way to circumvent the behavior. Since this answer, and Patric Bacon as stated in the official ESLint documentation points out a clear problem that could happen with it in certain scenarios: https://spin.atomicobject.com/2011/04/10/javascript-don-t-reassign-your-function-arguments/ – Remi Feb 13 '19 at 21:12
  • 3
    This answer should be the accepted answer since it's pointing to the official documentation and is well explained. Most of the other answers are like turning off the fire alarm! – Hamid Parchami Aug 14 '19 at 08:38
  • 2
    You may get "Local variable 'theElement' is redundant". – Phạm Tuấn Anh Sep 19 '19 at 05:15
  • What kind of naming scheme would you use for these new references? I absolutely hate putting 'My' in front of things here but it's really hard and even counter-intuitive to rename already aptly named parameters when creating a new reference. – GhostBytes Oct 14 '19 at 15:49
  • I just checked and `arguments[0]` was mutated. What am I doing wrong? `... theElement.expando = { p: 2 }; return arguments[0].expando; ...`. – mrmowji Jan 22 '20 at 21:19
  • 2
    *Modifying a property of the parameter doesn't mutate what that parameter refers to.*, I thought objects are passed by reference! Would you please elaborate? – mrmowji Jan 22 '20 at 21:41
  • 1
    The 'problem' you're trying to avoid here isn't actually a problem. You shouldn't use the `arguments` object, ever. Instead, you should use [rest parameters](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/rest_parameters). Thus no-param-reassign can be safely ignored. – jaquinocode Mar 12 '21 at 09:00
  • @mrmowji Objects are passed by reference, mutating the object does not change the reference, it changes only the properties of the object. https://jsfiddle.net/UselessCode/emao6txc/ – Useless Code Jan 09 '22 at 14:32
  • @UselessCode I think there's a misunderstanding about "mutation". JS objects are mutable. I think we should say something like *Modifying a property of the object parameter doesn't mutate the object reference*, because "what that parameter refers to" is simply an "object". Just to prevent confusion. – mrmowji Jan 11 '22 at 17:26
  • 1
    @mrmowji I agree it could have been worded better, I've updated the answer to hopefully be more clear. – Useless Code Jan 14 '22 at 11:01
161

As @Mathletics suggests, you can disable the rule entirely by adding this to your .eslintrc.json file:

"rules": {
  "no-param-reassign": 0
}

Or you can disable the rule specifically for param properties:

"rules": {
  "no-param-reassign": [2, { "props": false }]
}

Alternatively, you can disable the rule for that function:

/* eslint-disable no-param-reassign */
function (el) {
  el.expando = {};
}
/* eslint-enable no-param-reassign */

Or for a specific line only:

function (el) {
  el.expando = {}; // eslint-disable-line no-param-reassign
}
maxshuty
  • 9,708
  • 13
  • 64
  • 77
sfletche
  • 47,248
  • 30
  • 103
  • 119
  • 1
    Thank you. It seems most people find modifying the linter the best way to go. Applying this for a single line seems like the best trade off for me right now. – Lukas Feb 26 '16 at 16:18
  • 2
    That really make sense i.e. for nodejs express projects, where sometimes you might want to modify `res.session` straight away – David Aug 01 '16 at 09:10
  • If the problem is only with setting properties of function parameters as stated in the question, Gyandeep's answer below is much better. – Prashanth Chandra Jan 19 '17 at 04:57
  • thanks @hiehiuehue: looks like they took it down. removing link. – sfletche Jan 24 '21 at 18:49
23

You can override this rule inside your .eslintrc file and disable it for param properties like this

{
    "rules": {
        "no-param-reassign": [2, { 
            "props": false
        }]
    },
    "extends": "eslint-config-airbnb"
}

This way rule is still active but it will not warn for properties. More info: http://eslint.org/docs/rules/no-param-reassign

Gyandeep
  • 12,726
  • 4
  • 31
  • 42
19

The no-param-reassign warning makes sense for common functions, but for a classic Array.forEach loop over an array which you intend to mutate it isn't to appropriate.

However, to get around this, you can also use Array.map with a new object (if you are like me, dislike snoozing warnings with comments):

someArray = someArray.map((_item) => {
    let item = Object.assign({}, _item); // decouple instance
    item.foo = "bar"; // assign a property
    return item; // replace original with new instance
});
Vimal CK
  • 3,543
  • 1
  • 26
  • 47
Justus Romijn
  • 15,699
  • 5
  • 51
  • 63
13
function (el) {
  el.setAttribute('expando', {});
}

Everything else is just ugly hacks.

avalanche1
  • 3,154
  • 1
  • 31
  • 38
2

Those wishing to selectively deactivate this rule might be interested in a proposed new option for the no-param-reassign rule that would allow a "white list" of object names with respect to which parameter reassignment should be ignored.

RH Becker
  • 1,692
  • 1
  • 14
  • 11
2

Following the documentation:

function (el) {
  const element = el
  element.expando = {}
}
rosencreuz
  • 1,276
  • 10
  • 21
Victor Santos
  • 509
  • 4
  • 5
2

An updated alternative if you don't really want to disable the rule, could be something like:

function (el) {
  Object.defineProperty(el, 'expando',{
    value: {},
    writable: true,
    configurable: true,
    enumerable: true
  });
}

Reference : https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty

mgasp
  • 21
  • 1
1

You can also use lodash assignIn which mutates the object. assignIn(obj, { someNewObj });

https://lodash.com/docs/4.17.2#assignIn

Jazzy
  • 6,029
  • 11
  • 50
  • 74
1

Based on the documentation at eslint (https://eslint.org/docs/2.0.0/rules/no-param-reassign), I solved it using the below written configuration in the .eslintrc.js file.

rules: {
  "no-param-reassign": [2, {"props": false}]
},
Avijit Majhi
  • 508
  • 9
  • 15
0

You can use methods for updating data. Eg. "res.status(404)" instead of "res.statusCode = 404" I found the solution. https://github.com/eslint/eslint/issues/6505#issuecomment-282325903

/*eslint no-param-reassign: ["error", { "props": true, "ignorePropertyModificationsFor": ["$scope"] }]*/

app.controller('MyCtrl', function($scope) {
  $scope.something = true;
});
0

You can use spread operator to create a copy and change only 1 property.

function (el) {
  var editedEl = { ...el, expando: {} };
}

and then just keep using editedEl.

hubertinio
  • 44
  • 5
-1

You can use:

(param) => {
  const data = Object.assign({}, param);
  data.element = 'some value';
}
2540625
  • 11,022
  • 8
  • 52
  • 58
Oliver
  • 45
  • 6
  • 1
    Doesn't this only modify the copy (how is that valuable)? – 2540625 Dec 06 '16 at 03:23
  • 1
    This is not really a solution because this means *avoiding* param reassign by creating a new object while I explicitly need to not create a new object but modify the original one. – Lukas Dec 10 '16 at 09:34
  • I prefer not to modify params, but you can also use Object.defineProperty() if you do this way the linter won't throw you an error. – Oliver Dec 12 '16 at 14:47
  • This wouldn't work at all. You are making a copy of the variable, copying properties from it onto a new object, modifying a property and then not returning it, so nothing happens. Even if you did return it, you are returning an object, not the a DOM element like what was passed in. On top of that, [`Object.assign`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign) is for copying from an object to a target object. Trying to copy from a DOM element like that results in an empty object. – Useless Code Feb 22 '17 at 18:16
  • Plus `Object.assign` won't work appropriately if you're trying to re-assing a property on Object with circular references (a socket connection for example). `Object.assign` by default is a shallow copy, and deep cloning is very frowned upon because of the performance hit. – ILikeTacos Mar 01 '18 at 21:08