CODEWITHSUNDEEP
javascriptpromiseq
4m1r
4m1r

Reputation: 12552

Reducing all JS Promises before resolving to next then

I can't seem to find the right solution to this problem. I have a promise with a bunch of thenables and in one of the resolvers, I want to pause and do a series of work on an array (using promises), and I don't want this to actually resolve until the series of promises have completed. This example seems to get me pretty close, but I think I might have created an anti-pattern.

http://taoofcode.net/promise-anti-patterns/

// async_series.js
// example of using reduce and promises and closure to create async results in series

var q = require('Q');
var results = [1, 2, 3, 4, 5];

function workCollection(arr) {

    return arr.reduce(function(promise, item, index) {

        return promise.then(function(result) {

            return (function(i, r, idx){
                setTimeout(function(){
                    console.log('item', i, 'result', r, 'index', idx);
                }, 1000 * idx);
                return true
            })(item, result, index);

        });

    }, q().then(function(){return true}));

}

    q()
        .then(function(){
            console.log('start');
        })
        .then(function(){
            workCollection(results)
        })
        .then(function(){
            console.log('done');
        })
        .catch(function(err){
            console.log(err);
        });

Kind of works, but the output is:

start
done
item 1 result true index 0
item 2 result true index 1
item 3 result true index 2
item 4 result true index 3
item 5 result true index 4

as opposed to this (intended):

start
item 1 result true index 0
item 2 result true index 1
item 3 result true index 2
item 4 result true index 3
item 5 result true index 4
done

Suggestions greatly appreciated. Plz comment before down voting.

Upvotes: 0

Views: 427

Answers (4)

Bergi
Bergi

Reputation: 664559

There's multiple things:

  • as already mentioned, you need to return the (promise) result of `` from the then callback, otherwise the chain won't wait for it
  • q().then(function(){return true}) should simply be q(true)
  • The IIFE is useless. If you need a closure scope for anything, the reduce callback already provides it.
  • You can't return true and expect anything to wait for the setTimeout. You'll need to promisify it (wrap it in a new Q.Promise(…)), or just use Q.delay.

All in all, your code should look like this:

var q = require('Q');

function workCollection(arr) {
    return arr.reduce(function(promise, item, index) {
        return promise.then(function(result) {
            return Q.delay(result, 1000);
        }).then(function(result) {
            console.log('item', item, 'result', result, 'index', index);
            return true;
        });
    }, q(true));
}

q().then(function(){
   console.log('start');
   return [1, 2, 3, 4, 5];
}).then(workCollection).then(function(result) {
   console.log('done', result);
}, function(err){
   console.error(err);
});

Upvotes: 1

mido
mido

Reputation: 25034

the mistake you are making is( other than not returning workCollection promise) setTimeout is not promisified, you can fix it like:

var q = require('Q');
var results = [1, 2, 3, 4, 5];

function delayedCall(i, r, idx){
  return q.Promise(function(resolve, reject){  
    setTimeout(function(){
      console.log('item', i, 'result', r, 'index', idx);
      resolve(true); 
    }, 1000 * idx);
  });
}

function workCollection(arr) {
  return arr.reduce(function(promise, item, index) {
    return promise.then(function(result) {
      return delayedCall(item, result, index);
    });
  }, q(true));
}

q().then(function(){
  console.log('start');
}).then(function(){
  return workCollection(results)
}).then(function(){
  console.log('done');
}).catch(function(err){
  console.log(err);
});

fiddle demo, I have used Q.defer instead of Q.Promise but rest of code is same

Upvotes: 1

Tyler
Tyler

Reputation: 11509

You can achieve this with promise.all(). Just push each of your array promises into an array (often name deferred) and then return promise.all(deferred)

Documentation here: https://github.com/kriskowal/q/wiki/API-Reference#promise-for-array-methods

Update

Rather than edit your fiddle, I created one that uses deferred, which is what you should be using in this case. https://jsfiddle.net/twler/aujqcmhv/1/

Hopefully it will clear up a bit about promise objects, as they can be confusing.

Upvotes: 1

Bhabishya Kumar
Bhabishya Kumar

Reputation: 731

I think you are getting done right after start as you are not returning result of workCollection(results) in your then so its getting resolved with undefined right away. Try this

.then(function(){
    return workCollection(results)
})

Also, as suggested, go with Promise.all if the order doesn't matter otherwise your Promise Waterfall approach as also described here is fine.

Upvotes: 1

Related Questions