Comment by eurasiantiger | Hacker News Reader

Comment by eurasiantiger | Hacker News Reader

Parent Full thread

Yeah, but the pattern of awaiting in an async parallel loop in general reduces to what I wrote, does it not?

The only case where you would need to await inside the mapper function is when you would further process some asynchronously returned value. This seems like it would add bloat and do unnecessary sync processing inside the async function. It is highly advisable to separate the concerns using function composition, e.g.

    await Promise.all(
      arr.map(async item => handleItem(
        await item()
      ))
    );

I’m all for enabling ”await arr.map()” with implied Promise.all(), heh. There actually was a proposal for that under the ”await*” syntax in 2014.

One might be tempted to use the thenable interface (don’t!):

    await Promise.all(
      arr.map(item => item()
          .then(handleItem)
      )
    );

Seems like it would work, but it actually only waits for the promises returned from item() and not the .then() part! This introduces a race condition, where the last promised items to get resolved may have their then-callbacks run asynchronously after Promise.all() has already resolved, and other code has executed.

> the pattern of awaiting in an async parallel loop in general reduces to what I wrote, does it not?

No.

    async item => {await item()} // Promise<void>
    item => item() // Promise<ReturnValue<typeof item>>

In general that's how you use maps (by returning something), but here the intent was not to map an array, it's just how to implement an "async for".

> The only case where you would need to await inside the mapper function is when you would further process some asynchronously returned value

Which is what map is for. Also, once again, that was just how the async loop can be implemented. If anything you're saying that it's "a misuse" of map and thus we need an "async for"

    >  arr.map(async item => handleItem(
    >    await item()
    >  ))

That doesn't make any sense, there's still an `await` in the function’s body, so you're still handling it — and there's nothing wrong with that.

> await arr.map()

That's actually a good QoL improvement, but it doesn't address either of the points I'm talking about (only works on arrays + it requires a function)

> but it actually only waits for the promises returned from item() and not the .then() part

Wrong. The mapper function returns a Promise that resolves with the return value of handleItem. `await a.then(b).then(c)` awaits the 3 promises in row.

To achieve that race condition, you should not return nor await the return value of `then` inside the map method:

    await Promise.all(
      arr.map(item => {
        const p = item();
        p.then(handleItem); // "Race condition"
        return p;
      })
    );

I didn’t give any examples with block function bodies, of course they are different from what I expressed. You changed the semantics and claim wrongness :/

Your words:

> One might be tempted to use the thenable interface (don’t!):

The code that followed was fine and it had no race condition.

> Seems like it would work, but it actually only waits for the promises returned from item() and not the .then() part!

This is the wrong part. The blockness is irrelevant.

Reply on news.ycombinator.com