2

I have some code that works broadly as follows:

function getRegionUsers(region) {
    return new Promise((resolve) => {
        setTimeout(() => {
            if (region === "emea") {
                return resolve(["dave", "mike", "steve"]);
            }
            return resolve(["kiki", "maya", "yukiko"]);
        }, 1000);
    });
}

function sendMail(user, region) {
    console.info(`sendMail called: ${user} (${region})`);
}

async function test() {
    const regions = ["emea", "apac"];
    for (const region of regions) {
        const users = await getRegionUsers(region); // This line is my issue
        for (const user of users) {
            sendMail(user, region);
        }
    }
}
test();

You can ignore the first 2 functions, those just help illustrate the issue. The issue is the 3rd function which causes a linter error (eslint: no-await-in-loop) which makes sense as the await will hold up the execution of each iteration of my for loop.

The solution I keep seeing is to use Promise.all which is fine except I need to keep track of both user and region so I can pass it to the sendMail function. I therefore end up with something like this:

async function test() {
    const regions = ["emea", "apac"];

    const userPromises = regions.map((r) => getRegionUsers(r));
    const regionUsers = await Promise.all(userPromises);

    for (const [i, region] of regions.entries()) {
        for (const user of regionUsers[i]) {
            sendMail(user, region);
        }
    }
}
test();

This seems messy and inelegant since I essentially duplicate the regions for loop and it's harder to follow. It also gets messier once you deal with error catching or if you have another level or two of nested for loops.

Does anyone have a more elegant solution (other than just telling ESLint to ignore the line) even if it's just a neater way to write it?

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
Bob
  • 303
  • 1
  • 8
  • I think I fundamentally disagree with the rationale behind that eslint rule. There's no functional difference between doing the `await` in the loop and doing it via `Promise.all()`. – Pointy Jun 21 '22 at 13:14
  • what an odd linter! an await in a for loop is so common that it's virtually an answer to every question here where a noob misuses async/await in a array.forEach – Bravo Jun 21 '22 at 13:16
  • @Pointy the difference (with this example code) is that using `await` will take 2 seconds, using `Promise.all()` will take 1 because one will execute synchronously and the other will execute asynchronously, so the eslint rule is valid. – Bob Jun 21 '22 at 13:18
  • 1
    @Bravo only if the data is supposed to be sequentially processed. The rule is there as a reminder that a lot of things *do not need* to be processed sequentially `for(const id of whatever) arr.push(await fetchMoreData(id))` is perfectly fine to do as `arr = await Promise.all(whatever.map(fetchMoreData))`. The rule might sometimes be a bit it is not without merit. – VLAZ Jun 21 '22 at 13:18
  • 1
    @Bob uhh no, that is not correct. – Pointy Jun 21 '22 at 13:19
  • @Pointy run the code – Bob Jun 21 '22 at 13:22
  • Your code runs `test()` without an `await` and not from within an `async` function. – Pointy Jun 21 '22 at 13:22
  • @VLAZ - I get that - but unless eslint is analyzing what is being called, I can't see how the *error* could be determined – Bravo Jun 21 '22 at 13:23
  • OK I think I'm wrong; in this particular case, the difference is that the (imaginary) `.then()` callback for the `await` in the first case doesn't start the *second* timeout until the first one is done. So maybe there is a difference in general, though I'm not sure it's a "bad" thing in all cases. – Pointy Jun 21 '22 at 13:31
  • @Bravo I'd say *a lot of times* if you have an `await` in a loop, you make it non-sequential. Yes, there are cases where that's not possible and yes, ESLint cannot actually determine that (it's only examining the AST, it does not have any insight into the operations), however, however the rule would be correct more times than not in average code. If you have code where you clearly know you don't need the rule, then don't enable it. – VLAZ Jun 21 '22 at 13:34
  • fair enough @VLAZ - I wasn't arguing – Bravo Jun 21 '22 at 23:24

1 Answers1

2

I would map the regions with an async function. The async function always returns a promise (no need to add an explicit return statement). And the promises can be awaited with Promise.all.

async function test() {
    const regions = ["emea", "apac"];

    const promises = regions.map(async region => {
        const users = await getRegionUsers(region);
        for (const user of users) {
            sendMail(user, region);
        }
    });
    await Promise.all(promises);
}

This should fix the problem reported by no-await-in-loop. It is possible though that other ESLint rules, depending on your configuration, will start to complain.

GOTO 0
  • 42,323
  • 22
  • 125
  • 158