0

I was wondering why the following code is problematic to the CRA (and many other) js linter(s) by default. What I see here is a compact readable line of code. My guess is because the one liner below is not very scalable. Sometimes small simple chunks of code won't (shouldn't) need to get any bigger anyway. What other reasons are there to avoid writing simple one liners like this?

doSomething( prev => (prev.searchField = val, { ...prev }) );

Do I really need to do this to make the linter happy or are there other ways to keep the above one-liner?

doSomething(prev => {
  prev.searchField = val;
  return { ...prev };
});

To keep this succinct Lets not debate what is more readable just please answer why you think the linters warn this type of code with Unexpected use of comma operator no-sequences.

apena
  • 2,091
  • 12
  • 19
  • 2
    because someone made the rule. Normally these enforce more code to prevent typo mistakes. – epascarello Jul 27 '20 at 17:39
  • 2
    Almost everything a linter does is a matter of opinion. – Pointy Jul 27 '20 at 17:42
  • 1
    @Pointy unless it’s jslint and then it’s gospel – evolutionxbox Jul 27 '20 at 17:43
  • 1
    "Lets not debate what is more readable just please answer why you think the linters warn this type of code" Wouldn't saying why I think the linters warn that type of code lead to debate? – Heretic Monkey Jul 27 '20 at 17:44
  • 3
    While "readable" is subjective, "readable" and "compact" don't often go together. Either way, `doSomething(prev => ({...prev, searchField: val}))` achieves the same result and is much more commonly used. – Henry Woody Jul 27 '20 at 21:59
  • `prev.searchField = val;` is [mutating the state anyway, which is an anti-pattern in React](https://stackoverflow.com/q/37755997/1218980). – Emile Bergeron Jul 28 '20 at 15:54
  • It was a `setState` call I just changed the name to `doSomething` – apena Jul 28 '20 at 16:52

1 Answers1

3

The ESLint rules are very customisable, to allow teams that have specific guidelines to customise the warnings and lintings so that they can begin to use the tool without having to edit all their previous code.

However, CRA and a few others come with recommended linter settings that are generally "best practise" and also prevent beginners making mistakes.

Although you may think that simple one liner is readable, for a lot of people it isn't, and so that no-sequences rule is a part of that pre-selected rule-set that you happen to be using.

If you disagree with the recomendations, then you are able to turn it off easily.

no-sequences rule in EsLint docs

Luke Storry
  • 6,032
  • 1
  • 9
  • 22
  • 1
    Thanks! This is what I was looking for. The question got shut down for being opinion based even though I tried very hard to word it to get answers like yours. – apena Jul 27 '20 at 17:58
  • 2
    Yeah, everything is an opinion, StackOverflow makes it hard sometimes. I personally despise comma operators, but if your code has a load of them, then ESlint's composability is nice, so feel free to disable that. Generally though I would try to stick with the recommendations though, if only for it's easier for newbies who are used to what is becoming a slightly more standardised subset of JS. Also I think your example would be cleaner (and work with React.useState more nicely) if it were: `doSomething(prev => ({...prev, searchField: val}))` – Luke Storry Jul 28 '20 at 00:39