1

I am trying to implement my own Promise.all to prepare my interviews.

My first version is this

  Promise.all = function(promiseArray) {
    return new Promise((resolve, reject) => {
      try {
        let resultArray = []
  
        const length = promiseArray.length 
        for (let i = 0; i <length; i++) {
          promiseArray[i].then(data => {
            resultArray.push(data)
  
            if (resultArray.length === length) {
              resolve(resultArray)
            }
          }, reject)
        }
      }
      catch(e) {
        reject(e)
      }
    })
  }

However, the native Promise.all accepts not just arrays, but also any iterables, which means any object that has Symbol.iterator, such as an array or a Set or Map.

So something like this is going to work with the native Promise.all but not with my current implementation

function resolveTimeout(value, delay) {
  return new Promise((resolve) => setTimeout(resolve, delay, value))
}


const requests = new Map()
requests.set('reqA', resolveTimeout(['potatoes', 'tomatoes'], 1000))
requests.set('reqB', resolveTimeout(['oranges', 'apples'], 100))

Promise.all(requests.values()).then(console.log); // it works

I modified my Promise.all by first adding a check to see if it has Symbol.iterator to make sure it is an iterable.

 Promise.all = function(promiseArray) {
    if (!promiseArray[Symbol.iterator]) {
        throw new TypeError('The arguments should be an iterable!')
    }
    return new Promise((resolve, reject) => {
      try {

But the challenge is with how to iterate through the iterable. Current implementation is to get the length of if and doing it with a for loop via const length = promiseArray.length however only arrays have length property on it, other iterables or iterators like Set or Map.values() will not have that property available.

How can I tweak my implementation to support other types of iterables like the native Promise.all does

Joji
  • 4,703
  • 7
  • 41
  • 86
  • `Symbol.iterator` determines whether `for( ... of ... )` works, so I'd say just take advantage of that. Instead of testing the the symbol, just try to run the code you want to run anyway using `try { for( of ) {}} catch (err) { ... }`. You can keep a tally of how many things you're handling inside your for loop, no need for a `.length` property. However, you do need to return a pure array, with the same ordering as you start (not resolved) each promise (as per @derpirscher's comment). – Mike 'Pomax' Kamermans Oct 11 '21 at 21:36
  • 2
    You have a bigger problem with your approach: Once resolved, `Promise.all` must return an iterable which contains the result in the same order as the input promises. With your approach, you are just pushing results once they are available. Thus, the order may be completely random ... – derpirscher Oct 11 '21 at 21:37

2 Answers2

1

But the challenge is with how to iterate through the iterable.

Just use for/of to iterate the iterable.

Current implementation is to get the length of if and doing it with a for loop via const length = promiseArray.length however only arrays have length property on it, other iterables or iterators like Set or Map.values() will not have that property available.

Just count how many items you get inside the for/of iteration.


Here's a more detailed explanation of the implementation below

If you look at the spec for Promise.all() it accepts an iterable as its argument. That means that it must have the appropriate iterator such that you can iterate it with for/of. If you're implementing to the spec, you don't have to check if it is an iterable. If not, the implementation will throw and thus reject with the appropriate error (which is what it should do). The Promise executor already catches exceptions and rejects for you.

As has been mentioned elsewhere, you could use Array.from() on the iterable to produce an actual array from it, but that seems a bit wasteful because we don't really need a copy of that data, we just need to iterate it. And, we're going to iterate it synchronously so we don't have to worry about it changing during iteration.

So, it seems like it would be most efficient to use for/of.

It would be nice to iterate the iterable with .entries() because that would give us both index and value and we could then use the index to know where to put the result for this entry into the resultArray, but it does not appear that the spec requires support of .entries() on the iterable so this implementation makes do with just a simple iterable with for (const p of promiseIterable). The code uses its own counter to generate its own index to use for storing the result.

Likewise, we need to produce as the resolved value of the promise we're returning an array of results that are in the same order as the original promises.

And, the iterable does not have to be all promises - it can also contain plain values (that just get passed through in the result array) so we need to make sure we work with regular values too. The value we get from the original array is wrapped in Promise.resolve() to handle the case of a plain value (not a promise) being passed in the source array.

Then, lastly since we are not guaranteed to have a .length property, there is no efficient way to know in advance how many items are in the iterable. I work around that in two ways. First, I count the items as we go in the cntr variable so when we're done with the for/of loop, we know how many total items there were. Then, as the .then() results come in, we can decrement the counter and see when it hits zero to know when we're done.

Promise.all = function(promiseIterable) {
    return new Promise((resolve, reject) => {
        const resultArray = [];
        let cntr = 0;

        for (const p of promiseIterable) {
            // keep our own local index so we know where this result goes in the
            // result array
            let i = cntr;
            
            // keep track of total number of items in the iterable
            ++cntr;
            
            // track each promise - cast to a promise if it's not one
            Promise.resolve(p).then(val => {
                // store result in the proper order in the result array
                resultArray[i] = val;
                
                // if we have all results now, we are done and can resolve
                --cntr;
                if (cntr === 0) {
                    resolve(resultArray);
                }
            }).catch(reject);
        }
        // if the promiseIterable is empty, we need to resolve with an empty array
        // as we could not have executed any of the body of the above for loop
        if (cntr === 0) {
            resolve(resultArray);
        }
    });
}

FYI, you can see some of this logic in the ECMAScript specification for Promise.all() here. Also, make sure to look at PerformPromiseAll.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

The easiest and most practical tweak would be to copy your iterated results into an array you control. Array.from is a good match, as on MDN (emphasis mine):

The Array.from() static method creates a new, shallow-copied Array instance from an array-like or iterable object.

[...]

Array.from() lets you create Arrays from:

  • array-like objects (objects with a length property and indexed elements); or
  • iterable objects (objects such as Map and Set).

This might also help by giving your inputs clear indices, which (as derpirsher mentions in the comments) will be necessary to ensure that your all implementation returns results in order even if they may complete out of order.

Of course, as you're doing this to better prepare for interviews, you may choose to avoid helpful functions like Array.from and favor a homemade implementation based on for ( ... of ... ) to check your understanding, as Mike 'Pomax' Kamermans mentions in the comments. That said, once in a real development role, I hope you would use Array.from (and the built-in Promise.all) as much as possible to reduce duplication and edge cases.

Jeff Bowman
  • 90,959
  • 16
  • 217
  • 251
  • Why would you use `Array.from(foo)` over `[...foo]`? – Robo Robok Oct 11 '21 at 23:09
  • No strong personal reason; more thorough answers are [available on SO](https://stackoverflow.com/a/40549565/1426891). `[...foo]` doesn't work with Array-like objects with a `length`, though admittedly neither does `Promise.all`. `Array.from` is more friendly to polyfill, though both Promises and spread elements were standardized in ES6/ES2015, so they're both long-standard and probably fair game. TBH I think I just had `Array.from` in my head as a buffer from both iterables and non-Array `length`-containing objects like [NodeList](https://developer.mozilla.org/en-US/docs/Web/API/NodeList). – Jeff Bowman Oct 11 '21 at 23:23
  • But `[...foo]` works with NodeList :) Try `[...document.querySelectorAll('div')]` - it does the job! – Robo Robok Oct 11 '21 at 23:52
  • It does...on modern Chrome :) But only because [NodeList iterability](https://www.chromestatus.com/feature/5691608351637504) was added in Chrome 51 (2016-05), trailing [the introduction of `Array.from` in Chrome 45 (2015-09)](https://www.chromestatus.com/feature/6732923508097024) by [9 months](https://en.wikipedia.org/wiki/Google_Chrome_version_history). Edge [didn't even support `NodeList.forEach` until Edge 16](https://caniuse.com/mdn-api_nodelist_foreach) (2017-10-08). It's [unclear if Edge ever got support](https://stackoverflow.com/q/47287344/1426891) before switching to Chromium in 2019. – Jeff Bowman Oct 12 '21 at 00:13
  • 1
    @JeffBowman - The history of `for/of` iterability for nodeList or HTMLCollection is described [here](https://stackoverflow.com/questions/22754315/for-loop-for-htmlcollection-elements/22754453#22754453). Chrome and Firefox had it in Dec 2016. In Dec 2017, Edge had it for nodeList, but not HTMLCollection. By Nov 2018, Edge finally had it for HTMLCollection (before they transitioned to Chromium). – jfriend00 Oct 12 '21 at 00:26