no-param-reassign - Array.prototype.reduce and alike exceptions

[langpavel]

Hi and thanks everybody who participated on no-param-reassign!
I know, this is a duplicate of #8007 and #6339. This is a lobby.

Summary

no-param-reassign is pretty useful rule. I'm not breaking this rule all time except if I write code using this perfect function:
Array.prototype.reduce(function(accumulator, currentValue) => accumulator, initialAccumulator )

Currently there is no way to hint no-param-reassign rule to relax reduce function accumulator in perfectly finest way.

tl;dr

I really ❤️ no-param-reassign rule. But there is at least one edge case where assigning to passed variable is perfectly fine, possibly the best way to write it down — someArray.reduce() function.
I'm sure that everyone knows this function, using it time to time for transforming data from one shape to another.

I see misunderstanding of reduce with no-param-reassign rule leading to wasting a lot of resources. It is so simple. Just use spread operator for copying result of previous iteration:

// this code is perfectly fine, but violates rule:
const firstNameCount = users.reduce((firstNameCount, user) => {
  firstNameCount[user.firstName] = (firstNameCount[user.firstName] || 0) + 1;
  return firstNameCount;
}, {});

// this code is performing poorly, wasting memory and CPU, but fits rule :-(
const firstNameCount = users.reduce((acc, user) => {
  const result = { ...acc };
  result[user.firstName] = (acc[user.firstName] || 0) + 1;
  return result;
}, {});

This use case is so special and simple that there is no need for more similar functions. This is the pattern in fact. A coding primitive,
so special that famous framework Redux is mostly about thinking about this one function, deriving all the possibilities from using this pattern!

If you need more…

This is so special case that it can have it's own syntax, but, you know, this is not needed in javascript. But I can imagine language expression construct like this:

reduce <Iterable> into <lvalue> [ = <rvalue> ] with <identifier> [, <identifier>] as <statement using <lvalue> >

To be more concrete:

const firstNameCount = users.reduce((firstNameCount, user) => {
  firstNameCount[user.firstName] = (firstNameCount[user.firstName] || 0) + 1;
  return firstNameCount;
}, {});

can be written like this:

const firstNameCount =  reduce users into firstNameCount = {} with user as {
  firstNameCount[user.firstName] = (firstNameCount[user.firstName] || 0) + 1;
}

const firstNameCount = reduce users into firstNameCount = {} with user [, index] as {
  firstNameCount[user.firstName] = (firstNameCount[user.firstName] || 0) + 1;
}

PS: This is only illustrative example of language construct! I cannot imagine language except Pascal which want introduce this… 😄

Is Array.prototype.reduce only exception?

I thing no. But I percept it as language primitive so it should be only default exception.

[ilyavolodin]

This would also apply to reduceRight, however the problem with creating such enhancement is that we don't know if you are calling reduce on the array, or on your custom made object. We don't track variable types in ESLint, so we don't really have a good way to figure out if the current reduce function should be excluded or not.

[langpavel]

We don't track variable types in ESLint...

Yes, but it is possible to temporary disable rule by name of calling method?
I know this is definitely not bulletproof but can be acceptable exception..
For example if I define reduce and reduceRight in rule config as methods which flags function in argument for exception.. I think this should be possible.

[ilyavolodin]

That's pretty error-prone way of doing it:

var a = {
  reduce: function() { console.log("I'm not the reduce you are looking for"); }
}

[platinumazure]

@ilyavolodin It's the same way we've implemented the rule, though, is it not?

[ljharb]

Regarding the claim in the OP "// this code is perfectly fine, but violates rule:" - the whole point is that it's not fine, it mutates the accumulator. Certainly that entire reduce operation is pure even though the reducer is not, but there's no way for eslint to know for certain that a) it's Array.prototype.reduce, and b) you've passed a new value in as the initial accumulator.

Lacking that certainty, the proper solution is to enforce no mutation of arguments, or, for you the developer (who does have that certainty) to use an override comment.

[not-an-aardvark]

Thanks for your interest in improving ESLint. Unfortunately, it looks like this issue didn't get enough support from the team and so I'm closing it. While we wish we'd be able to accommodate everyone's requests, we do need to prioritize. We've found that issues failing to reach consensus after a long time tend to never do it, and as such, we close those issues. This doesn't mean the idea isn't interesting, just that it's not something the team can commit to.

[daaain]

For anyone who is just looking for the quick solution, this is how you disable the warning:

const firstNameCount = users.reduce((firstNameCount, user) => {
  firstNameCount[user.firstName] = (firstNameCount[user.firstName] || 0) + 1; // eslint-disable-line no-param-reassign
  return firstNameCount;
}, {});