Proposal: Remove "props": true from no-param-reassign

[ianmcburnie]

Current rule:

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

Proposed rule:

"no-param-reassign": [2]

Can we go with the default eslint config for no-param-reassign? The default sets props to false. This will still disallow reassignment of the param, but will allow reassignment of properties on that param (which is a fairly common thing to do on the client).

For example, right now this is will be a lint error:

function foo(bar) {
    bar.prop = "value";
}

Which seems a lot less harmless than the following:

function foo(bar) {
    bar = 13;
}

So is there a reason why we decided to disable property re-assignment too? Perhaps it's more of an issue for server side script than DOM manipulation...?

For a more real life example, this is where I ran into this error:

wrapCheckbox.addEventListener('change', function(e) {
    emitters.forEach(function(emitter) {
        emitter.config.wrap = e.target.checked;
    });
});

I would have to rewrite it like this, with the current rule:

wrapCheckbox.addEventListener('change', function(e) {
    emitters.forEach(function(emitter) {
        var theEmitter = emitter;
        theEmitter.config.wrap = e.target.checked;
    });
});

Further reading:

• https://stackoverflow.com/questions/35637770/how-to-avoid-no-param-reassign-when-setting-a-property-on-a-dom-object
• https://spin.atomicobject.com/2011/04/10/javascript-don-t-reassign-your-function-arguments/
• https://eslint.org/docs/rules/no-param-reassign

[senthilp]

Agree, it makes sense. @mwoo shall I go ahead and make the change?

[mikewoo200]

That also makes sense to me. Thanks, @ianmcburnie for bringing this up.

I have made the change and published new version (@0.1.6).

http://registry.npmjs.org/eslint-config-ebay

[ianmcburnie]

Thanks guys.