getify / eslint-plugin-proper-arrows

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

Add option to enable only in blocks where brackets are present #1

Closed willishq closed 5 years ago

willishq commented 5 years ago

[EDITORS NOTE: This issue was transferred from another repository, so it may no longer be valid, still TBD]

I personally can see merit to using anonymous functions using the shorthand syntax when doing simple calculations for filters and mapping arrays, for instance.

I would like the ability to only apply the rule when the arrow function contains curly brackets, so this should be fine

[1, 2, 3].map(x => x * 2).filter(x => x < 4);

But this wouldn’t be, without a reference to this


new Array(6).fill(undefined).map((_, key) => {
   let thing = DoSomething(key);
   return thing();
}

I’m in two minds as to what to do about returning objects:


x => ({
  …
})

I’ve never used this syntax because it’s too messy in my opinion, but I think it should be part of the rule options too, which may complicate things because of the brackets.

getify commented 5 years ago

I would like the ability to only apply the rule when the arrow function contains curly brackets

Good idea, thanks!

I can imagine this mode opted-on via a separate optional configuration, like this:

"@getify/arrow-require-this/all": ["error","nested","block"]

"@getify/arrow-require-this/all": ["error","always","block"]

If "block" is on, this rule would effectively just ignore any arrow function that only has a concise expression body.

Bikeshedding: I don't love the word "block" here, since the { } on a function is not technically a "block" in the AST sense. A more accurate name would be "non-concise" or "full-body"... but those aren't great.

which may complicate things because of the brackets.

I doubt this will create an issue because even though the { } look like a block, the parser produces entirely different AST nodes which should be easy to distinguish.

Can you elaborate though on why you think x => x * 2 shouldn't need the rule applied, but x => ({ x: x * 2 }) should? IOW, those seem like the same kind of arrow function to me, so I'm not seeing why they should be distinguished in the rule. But perhaps I'm missing something?

willishq commented 5 years ago

I believe they are the same type of arrow function too, although the second impression, returning an object, could be messy and unclear. I think it’s something that I’d like to be able to disable / enable linting for purely for personal aesthetics rather than any standard, and as such, if we have the ability to enable and disable linting options for one kind, it makes sense to enable more granular control, such as returning objects from arrow functions in that way.

getify commented 5 years ago

Do you mean you want to be able to disallow the concise object return? Or do you mean you want to be able to require 'this' inside a concise object return?

willishq commented 5 years ago

In the instance where you enable the ability to use single-line arrow functions without the this requirement, I think you should be able to specify at a more granular level what constitutes a single-line arrow function, and if the arrow return does so.

for instance, allowing the object return enables you to write this:


const wtf = a => ({
    b: function(b) {
        return b * this * 3.14;
    }.bind(a),
    c: a + a,
    d: e =>  e * wtf(this.value).b(e)
});

which would be totally valid but should it be?

getify commented 5 years ago

I'm trying to figure out what you think would be different about that vs:

const wtf = a => [
   function(b) {
      return b * this * 3.14;
   }.bind(a),
   a + a,
   e => e * wtf(this.value)[0](e)
];

Would you want only yours to be flagged, or both of these?

In the case of "nested" mode, both examples would be allowed since both of them have the e => ..this.. arrow function nested. If that wasn't present, or if "always" mode was on, then both of the examples would be flagged.

So what I'm trying to figure out is, with respect to how this particular plugin handles this identification (and, optionally, nesting), is there any difference you see between your example and mine?

It feels like both should be treated the same (which is how the current plugin works), and that the { } of the object form (your example) is a bit of a red herring. But I just wanted to see if I was missing something?

getify commented 5 years ago

Or perhaps your notion of "single line" isn't really about a block or not, and more about whether nested/returned functions should be allowed? Or maybe that only one function may be returned, but not multiples?

Sorry for so many questions, just trying to clarify so we design something reasonable.

getify commented 5 years ago

I've transferred this issue over to this new project. It may not be valid anymore, I'm not sure. Please check out this package's "object-return" rule to see if it is helpful.

getify commented 5 years ago

I think I will close this issue for now, but please feel free to re-open it if you think it needs to be re-considered.