3

I am working with a broken codebase so I am trying to touch as little as possible. In particular, right now it's using TypeScript and ES6, and I need to launch an array of promises and wait for them all to finish before I move on with the code execution, regardless of if they resolve or reject. So this is the usecase for Promise.allSettled, which is only available in ES2020.

I tried the following implementation:

const myPromiseAllSettled = (promises) => new Promise((resolve, reject) => {
  const results = []
  const settle = (result) => {
    results.push(result)
    if (results.length === promises.length) {
      (results.every(value => value) ? resolve : reject)(promises)
    }
  }
  promises.forEach(promise => {
    promise
      .then(() => settle(true))
      .catch(() => settle(false))
  })
})

but I have only seen my own code when it comes to promises, so I would like to get some feedback on my implementation. Does it do what other developers would expect it to do? Especially when it comes to the arguments passed to resolve/reject; right now I only pass the array of promises and expect the developer to run a .then(promises => promises.forEach(...)) if they are interested in following up on the individual promises.

I also don't know ideally how I would handle the types with TypeScript here, since I am not so experienced with TypeScript as well. (Writing 'any' everywhere doesn't seem cool to me.)

Patrick Da Silva
  • 1,524
  • 1
  • 11
  • 15
  • 1
    the order is not preserved – marzelin Jan 12 '21 at 09:11
  • it doesn't return the same thing that `allSettled` returns (a promise that fulfills to an array of object with `status` and `value/reason` props) it just returns what has been passed to it. – marzelin Jan 12 '21 at 09:32
  • No, this is not a valid polyfill. It doesn't fulfill or reject with the proper values. And you don't even need to use the `new Promise` constructor. It should look [more like this](https://stackoverflow.com/a/30930421/1048572) or [that](https://stackoverflow.com/a/56255129/1048572) – Bergi Jan 12 '21 at 09:36
  • To me, this part here `(results.every(value => value) ? resolve : reject)(promises)` looks like you'd want `Promise.all` not `Promise.allSettled`. What's the point in waiting for all to finish, if you'll throw anyway, if a single one fails? Why not throw immediately? And why `resolve`/`reject` with an array of promises? `allSettled` returns you a list with which promises succeeded and which ones failed. That's why it waits for all to finish before returning the result. – Thomas Jan 12 '21 at 10:03

1 Answers1

2

it should look more like this:

const myPromiseAllSettled = (promises) => {
  const fulfilled = value => ({ status: "fulfilled", value });
  const rejected = reason => ({ status: "rejected", reason });

  return Promise.all([...promises].map(p => Promise.resolve(p).then(fulfilled, rejected)));
}

[...promises] to handle cases where promises is iterable but not an array. Promise.resolve(p) because the passed value may be not a Promise (or thenable).

If you simply want to be notified about the success, you can do something simpler:

const success = await Promise.all(promises).then(() => true, () => false);

Edit: Didn't like the way handled promises may be an iterable but no array. Adding a version that Array#maps over iterables:

function* map(iterable, callback) {
  for (const value of iterable) {
    yield callback(value);
  }
}

const myPromiseAllSettled = (promises) => {
  const fulfilled = value => ({ status: "fulfilled", value });
  const rejected = reason => ({ status: "rejected", reason });

  return Promise.all(
    map(
      promises,
      p => Promise.resolve(p).then(fulfilled, rejected)
    )
  );
}
Thomas
  • 11,958
  • 1
  • 14
  • 23
  • can you type that? – marzelin Jan 12 '21 at 10:44
  • @marzelin I don't understand the question, can I type what? – Thomas Jan 12 '21 at 12:08
  • @Thomas: So if I understood this correctly, your reasoning is that this Promise.resolve(p) just converts the settled promise p into a resolved promise that is identical? Won't that prevent Promise.all from rejecting? Because I would still like to be able to catch rejected promises in the array. I don't really understand what the second argument to .then does, I thought it took only one (the resolve callback)? – Patrick Da Silva Jan 12 '21 at 12:13
  • 1
    What I meant was can you provide TypeScript types for your code :) – marzelin Jan 12 '21 at 12:21
  • @PatrickDaSilva the second argument to `then` is for a rejection. It's equivalent of adding `catch` method. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then#parameters – marzelin Jan 12 '21 at 12:24
  • 1
    @PatrickDaSilva `Promise.resolve(p)` is in the case when one of the values in the passed array is a non-thenable object, like a string or number. If you try to do `"abc".then()` you get an error. But if you wrap the value in `Promise.resolve()` everything works smoothly (You can pass an array with plain values to `allSettled` or `Promise.all()`). – marzelin Jan 12 '21 at 12:28
  • @Thomas okay, I added types myself: https://www.typescriptlang.org/play?#code/C4TwDgpgBAtghmSATAPAFQHxQLxQN4BQUxUA2gNJQCWAdlANYQgD2AZlGgLoBc+UAzsDjAArv14AiViIA2rKjJkQkEgDRQAbnBkiIvNBU5QAvlAA+fQcLGSAThABWEAMbBlaqPbj9mNXgFFbW2ZbEwJjAgJQSCgABWCYKn4qVhB0LFxCEjJKWgYmNg4eOISkiHRDDHDI519BWBB45kT+CABBRQBlCGBgJSQcKHQoCAAPNxokfig4GhBSTgwACjBS1vES5qSUtMwAShwsLJJ7UVs6JpaIADptGSXSa6fVrfXOa-gwFcPNq+v7HwyDQQFZ7a7AAAWEBoSyI2RIWh00GwWCWeAEQlEGyksnkinc6kRuhMe1UcPhngg3l8PzRGOs2PsTlcBMp1Loxj2e3JUC5M2mlzKKE+yHSGAA3NUCEgXDI4PYoEpgFBRrxBclUihSIJbLQAObqGgiGAAIwgtkWktqNHqoxggxgjTW7S6PT6yiWoz24qAA – marzelin Jan 12 '21 at 12:50
  • 1
    @PatrickDaSilva `Promise.resolve(value)` converts every passed value into a `Promise`, like `String(value)` converts every passed value into a string. And about the second argument to `then`, `catch(fn)` is equivalent to `then(null, fn)` – Thomas Jan 12 '21 at 12:56
  • @Thomas: I will try to understand before I accept, but my intuition tells me that you have the right answer. The thing that I don't understand is that if `Promise.resolve(p)` creates a promise that resolves with return value p, I don't understand how that promise, as a thenable, can be `.then`ed in a way that it detects resolution or rejection of p when p is a promise. But if it works the way you say it does then this sounds like an elegant solution. The only thing I don't like about it right now is that it feels mystical to me haha – Patrick Da Silva Jan 12 '21 at 18:12