-
-
Notifications
You must be signed in to change notification settings - Fork 420
Description
Description
SonarQube/SonarCloud just added 68 rules from this plugin to their SonarWay ruleset, which is how I ended up here. While my main complaint is over there, my research led me down this rabbit hole and to @fisker suggesting to open an issue for rules that someone thinks are actually opinionated to have a discussion about it.
Looking at the original ticket and PR explanation, rules that were specifically stylistic or opinionated changes should not have been included. no-array-reduce and no-for-loop were not added as they are stylistic choices.
When the rule no-array-for-each was added, the main justification was stylistic: "for...of is more readable." Readability is almost always a personal preference. I find forEach significantly more readable than for...of, especially if the index is needed in the loop. "Github disallows this too" was added as a supporting argument, but this comment suggests it's inclusion there is for more org-specific reasons than any larger purpose.
for...of is not always faster than forEach, and can perform much worse as arrays get larger until you're in the millions of items range. With small arrays, the difference in performance is (usually) tiny. Being able to use control flow to break/continue is not a concern for most uses of forEach, and if those are necessary for the task then for...of is still an available tool. That becomes a "right tool for the job" decision best left up to the developer.
I have yet to see a scenario where typescript went weird inside forEach that didn't uncover a logical or programmatical issue that was at fault. I'm not saying that hasn't happened, but if it's not popped up my decade of heavy TS work it doesn't feel enough of a concern to complete disallow forEach.
The one scenario that can bubble up is that forEach isn't async-safe (like most Array iteration methods), but those scenarios are handled in a larger ruleset like @typescript-eslint/no-misused-promises.
Examples
Difficult to read & parse
// ❌
for (const [index, element] of list.entries()) {
console.log(`${element} of ${index}`);
}
Easy to read & parse
// ✅
list.forEach((element, index) => {
console.log(`${element} of ${index});
}Additional Info
No response