CODEWITHSUNDEEP
javascriptasynchronousrecursionpromise
Steven Mercatante
Steven Mercatante

Reputation: 25315

Fire Promise.all() once all nested promises have resolved

I'm trying to recursively fetch all the comments for a Hacker News story using their Firebase API. A story has a kids property, which is an array of IDs representing comments. Each comment can have its own kids property, which points to its children comments, and so on. I want to create an array of the entire comment tree, something that looks like:

[{
  'title': 'comment 1', 
  'replies': [{
    'title': 'comment 1.1'
  }, {
    'title': 'comment 1.2'
    'replies': [{
      'title': 'comment 1.2.1'
    }]
  }]
}]

I thought I could do this using the following function:

function getItem(id) {
    return api
        .child(`item/${id}`)
        .once('value')
        .then((snapshot) => {  // this is a Firebase Promise
            let val = snapshot.val()

            if (val.kids) {
                val.replies = val.kids.map((id) => getItem(id))
            }

            return val
        })
}

And then be notified once the entire comment tree has been fetched by using:

getItem(storyId)
    .then(story => {
      // The story and all of its comments should now be loaded
      console.log(story)
    })

What ends up happening is Promise.all().then() fires once the first level of comment promises have resolved (which makes sense since all of the commentPromises have resolved.) However, I want to know once all the nested promises have resolved. How can I do this?

Upvotes: 6

Views: 683

Answers (2)

user663031
user663031

Reputation:

There is no need for a wrapper promise as in other answers; that is the "explicit promise constructor anti-pattern". Streamlining a bit more, you can do the following:

function getItem(id) {
  return api
    .child(`item/${id}`)
    .once('value')
    .then(snapshot => {
      const val = snapshot.val();
      return Promise.all((val.kids || []).map(getItem))
        .then(kidsVals => {
          val.replies = kidsVals; 
          return val; 
        });
      );
    });
}

There is also no need for any explicit rejection handling. Rejections will propagate up naturally to the top level (assuming that's what you want).

Upvotes: 7

Vivek Athalye
Vivek Athalye

Reputation: 2979

IMP: Please refer torazaburo's answer. It's much better than mine.

=========

I think this should work:

function getItem(id) {
    return new Promise(function(resolve, reject) {
        api
        .child(`item/${id}`)
        .once('value')
        .then((snapshot) => {  // this is a Firebase Promise
            let val = snapshot.val()

            if (val.kids) {
                let replies = val.kids.map((id) => getItem(id))
                Promise.all(replies).then(replies => {
                    val.replies = replies // <<<<<< try this
                    resolve(val) // we want to resolve val, not val.replies
                }, err =>{
                    reject(err)
                })
            } else {
                resolve(val)
            }
        })
        .catch((err) => { // if there was some error invoking api call we need to reject our promise
            reject(err);
        })
    }
}

Edit: Setting val.replies inside then

Upvotes: 3

Related Questions