Skip to content

Remove no-array-for-each from unopinionated preset #2820

@sickhippie

Description

@sickhippie

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions