getify / eslint-plugin-proper-arrows

ESLint rules to ensure proper arrow function definitions
MIT License
308 stars 14 forks source link

Include delegations under trivial functions #28

Closed ghost closed 3 years ago

ghost commented 4 years ago

It'd be nice if we could include the functions x => f(x) under the definition of trivial. For example, if we want to call Array.map(x => f(x)) sometimes there are optional extra parameters of f that don't necessarily coincide with the parameters Array.map passes, and you need to delegate like this.

Another good example is for delegations that use methods, e.g. x => x.f().

Also constructors: x => new T(x)

getify commented 4 years ago

I'm not sure I see these as "trivial", though I could see a case to be made for there to be a separate category for them.

ghost commented 4 years ago

Honestly, I feel that the term "trivial" may be a bit too vague, then. To me, the amount of work this function is doing is simple enough that it doesn't require an additional name, and hence would be trivial under the lint.

getify commented 4 years ago

The term "trivial" refers to visual and cognitive triviality... aka "readability".

For example, () => x is much more visually simple, unambiguous, and ultimately readable at a glance than () => new foo.bar(y * 3).

I was just indicating that I could see () => fn(x) as something someone may want to allow, though () => new foo.bar.baz.bam.whatever(x + 1,w * 2,y / z) to me is not the same. But neither of those is as readable-trivial as () => x, IMO.

ghost commented 4 years ago

That's extremely fair. I guess that part of the sense to me is that the goal of the lint is to ensure proper function names for debugging purposes, and certain kinds of functions don't need to have that name because even if they fail, it's clear what failed.

I see no benefit of adding function createWhatever() { return to the beginning of that example because I should be commenting what it's doing in the code anyway.

getify commented 4 years ago

Like I said, I could see a case for another category of allowable arrow functions (tho it would have to stay defaulted to disallowing, to prevent breaking changes).

My concern is, where do we draw the line?

x => f(x) is pretty reasonable, but x => foo.bar(x * 2 + 3,y,...whatever) is not, IMO, deserving of any other label than "complex", which is to say that you should just not use this plugin if you like allowing those.

If we can come up with a reasonable and tight definition of what's narrowly allowed in this class of arrow function, I'm open to adding it.