New Rule: no-async-in-foreach

[Woodz]

Rule details

Warn about using async within Array.forEach

Related ECMAScript feature

ECMAScript 2017 - await/async

What type of rule is this?

Warns about a potential problem

Example code

const events = ['Hello world!', 'Goodbye world'];
async function publishEvent(event) {
  sleep(1000);
  console.log(event);
}

events.forEach(async (event) => await publishEvent(event));

Why should this rule be in the core instead of a plugin?

It is dangerous to use async inside forEach because the promises side-effects will not be evaluated before the forEach completes.

In environments such as AWS Lambda, this is even worse, because the execution environment freezes pending promises on the event loop and may resume the promises if the execution environment is re-used (see https://levelup.gitconnected.com/avoiding-the-pitfalls-of-async-node-js-functions-in-aws-lambda-941220582e7a). This will result in the side-effect of the async forEach either:

1. Sometimes executing
2. Never executing
3. Sometimes executing in a different Lambda call

Given the danger of this oversight, I think this should be a core rule to protect junior and inexperienced developers from this easy mistake.

Participation

• I am willing to submit a pull request to implement this rule.

Additional comments

I know this issue is not related to new ECMAScript features, but given the severity of the issue, I really think it should be added to core.

This topic was previously brought up in #12576 but they proposed solving it by extending no-await-in-loop which has a different purpose - it is designed to optimise execution by moving sequential to parallel execution. This issue is different as this is far worse than suboptimal execution so therefore I think this should be a separate rule.

[nzakas]

Thanks for the suggestion. This is a bit too narrow for the core. The problem is that most array methods that expect callbacks will produce unexpected results when the callback is an async function, so there’s not a good reason to single out forEach().

We generally don’t like to add rules to the core that essentially type-check parameters for various methods. It’s a lot of work and it’s also prone to false positives because we don’t have type information to be sure that a forEach() method is being called on an array vs something else.

You can definitely create a custom rule if this is important for your work, but it’s not suitable to be included in the core.