CODEWITHSUNDEEP
javascripteslint
Christie
Christie

Reputation: 57

Why is ESlint happy for me to modify object properties as long as the object is reassigned? no-param-reassign

In ESLint, the no-param-reassign rule as documented here, forbids you from assigning the value of a function param.

This is to avoid having a function's arguments object be modified.

The correct way to code is to reassign the param to a local var and return the var. This is fine for some types, but it seems pointless for objects passed to a function.

For example, let's take this function;

function foo(param) {
    var copy = param; // This makes the linter happy
    copy.bar = 2;
    console.log('arg 0: ', arguments[0], 'param:', param, 'copy:', copy);
    return copy; // A pointless return, the original object has been modified.
}

let test = { bar: 1 };
foo(test); 
console.log(test); // Has been modified
test = foo(test); // a pointless reassignment, foo has already changed test.
console.log(test); // Same effect as previous function call.

To be fair, ESLint does allow you to turn this feature off with /*eslint no-param-reassign: ["error", { "props": false }]*/; but I have to wonder why?

The point of this rule is to get rid of mutability and keep the arguments object pure, but a simple reassignment of an object will not do this.

The only way to truly do that would be to deep clone the param and assign that to a function scoped variable.

Am I missing something here?

Upvotes: 4

Views: 1015

Answers (2)

thisisashwani
thisisashwani

Reputation: 1844

Barmar's answer really hits the point. I am still adding my two cents here because this rule is very important when it comes to large codebases. Starting with your example only, the job of ESLint is to point the wrong coding practices. We, as developers, can still fool the linter though in many ways!

The correct way to address this ESLint error in function foo would have been this -

function foo(param) {
    // This makes the linter happy, as well as the codebase :')
    const copy = { ...param, bar: 2 }; 
    console.log('arg 0: ', arguments[0], 'param:', param, 'copy:', copy);
    return copy; // No modifications to the original object
}

As you can see, deep clone of the param is not required.

Regarding your question about the why of this rule, have a look at this super-long thread running on Github discussing the very same question that you have mentioned - https://github.com/airbnb/javascript/issues/719. Hope it helps you learn something new and interesting :)

Side Note - Simply reassigning parameters also makes code hard to follow. It also seems a bad idea as it deoptimizes in many engines, specifically v8. To be honest, I am still reading more on this to understand this last line better. In case you want to read on this too, see here - https://github.com/airbnb/javascript/issues/641#issuecomment-167827978

Upvotes: 1

Barmar
Barmar

Reputation: 782702

The likely reason that it doesn't warn when you assign the parameter to a variable is that it would require complex data flow analysis. Suppose you have code like this:

function foo(param, flag) {
    var copy = flag ? param : {...param};
    copy.bar = 2;
    console.log('arg 0: ', arguments[0], 'param:', param, 'copy:', copy);
    return copy; // A pointless return, the original object has been modified.
}

Now it can't tell whether copy contains the same object as param or a clone, it depends on the value of flag.

Or something like this:

function foo(param) {
    var copy = param;
    var copy2 = copy;
    var copy3 = copy2;
    copy3.bar = 2;
    console.log('arg 0: ', arguments[0], 'param:', param, 'copy:', copy3);
    return copy3;
}

This would require keeping track of the entire chain of references to determine that copy3 is the same as param.

Tracking this isn't impossible, optimizing compilers often do it. But it may be overkill for a linter.

Upvotes: 4

Related Questions