CODEWITHSUNDEEP
javascriptasynchronouspromise
Kraken
Kraken

Reputation: 5860

Dynamically fill a Promise.all()

I'm trying to figure out why this bit of code isn't working. I'm looping through a dependency tree (deepObject), and for each one I want to run a function which returns a promise. I would then like for the next set of functions to happen after the resolution of all the promises, but the console logs inside the promise.all are not executing. ES6 is cool too, if you have a better approach, however I would also really like to know why this code is not working.

updated to add .catch (this had no effect)

for (let i = 0; i < keys.length; i++) {
    promisesArray.push(promiseFunction(deepObject[keys[i]]));
}
Promise.all(promisesArray).then(resolved => {
    console.log('dep promises resolved');
    console.log(`resolved: ${JSON.stringify(resolved)}`);
}).catch(err => console.error(new Error(err));

// promiseFunction is recursive
promiseFunction(obj) {
    return new Promise((resolve, reject) => {

        const keys = Object.keys(deepObj);
        for (let j = 0; j < keys.length; j++) {
            if (Object.keys(deepObj[keys[j]]).length) {
                // adding return statement to following line breaks it
                // that was already recommended, see threads
                promiseFunction(deepObj[keys[j]]);
            } else {
                const dep = new Package(keys[j]);
                console.log(`skan dep: ${keys[j]}`);
                dep.fetch().then(() => {
                    return anotherFunction();
                }).then(() => {
                    return Promise.all([
                        promise1(),
                        promise2(),
                        promise3(),
                    ]).then(resolved => {
                        if (resolved[0].propertyINeed) {
                            resolve(true);
                        }
                        resolve(false);
                    });
                }).catch(err => {
                    console.error(new Error(err));
                    process.exit(1);
                });
            }
        }
    });

I know this conversation - has been discussed - on here before

In the second link above the accepted answer suggests:

In case you are asynchronously filling the array, you should get a promise for that array, and use .then(Promise.all.bind(Promise)). If you don't know when you stop adding promises, this is impossible anyway as they might never all be resolved at all.

However I'm not using async to fill the array. Do I need this? What am I missing here?

update

Since I have error logging now in both the .then() as well as the .catch() it seems like something is going wrong unrelated to what happens inside of it.

Adding the return statement before promiseFunction(deepObj[keys[j]]); broke the recursion. I went from iterating over 173 objects to 68. Adding the catch logged no additional results. Code above is updated to share more of the recursive fn. When I run it, it seems to execute all the promises, but I have no way of knowing that. I'm most concerned with knowing 1. that the promise array for promise.all contains all the objects it should and 2. catching the moment when all of those promises for all the objects inside the recursed object are resolved.

Also, for the record, each one of these functions that return promises are all necessarily async. I have gone back through all of this multiple times in an attempt to simplify and remove any unnecessary promises. The task is just complex. There are a series of steps that must execute, they are async, and they are chained because they must be resolved in a specific order.

Upvotes: 4

Views: 8259

Answers (2)

Fabio Lolli
Fabio Lolli

Reputation: 889

Let's have a look at your promiseFunction...

for (let j = 0; j < keys.length; j++) {
    if (Object.keys(obj[keys[j]]).length) {
        //if you get in here, promiseFunction returns undefined, and it never gets resolved
        promiseFunction(obj[keys[j]]);
    } else {
        const dep = new Package(keys[j]);
        console.log(`skan dep: ${keys[j]}`);
        // and some more async hell that all resolves correctly
        //but even if it resolves correctly, you still have the if...
    }
}

To do the trick, just return your promiseFunction result in the if statement:

for (let j = 0; j < keys.length; j++) {
    if (Object.keys(obj[keys[j]]).length) {
        //if you get in here, you're saying this promise will depend on how this second promiseFunction will resolve
        return promiseFunction(obj[keys[j]]);
    } else {
        //omitted for brevity...
        resolve('your value');
    }
}

Now when you go into the if statement, you're basically telling the promise to solve based on whether that second promiseFunction has solved correctly - you're delegating the resolution to another Promise.

I never heard anybody give it this name, but you can think of this as Promise recursion :)

Upvotes: 0

Quentin
Quentin

Reputation: 943537

if (Object.keys(obj[keys[j]]).length) {
    promiseFunction(obj[keys[j]]);
} else {
    // and some more async hell that all resolves correctly

If Object.keys(obj[keys[j]]).length is true then you never call resolve or reject so the promise never resolves.

(Note that calling promiseFunction recursively creates a new promise which is never put into Promise.all).

Since some of your promises don't resolve, Promise.all won't either.

You probably need something more along the lines of:

var promises = [];
processObject(obj);
Promise.all(promises).then(etc etc);

function processObject(object) {
    for ( loop; over; object ) {
        if (condition) {
             processObject(object[something]);
        } else {
             promises.push(new Promise( (res, rej) => {
                 // so something async
             });
        }
    }
}

Upvotes: 4

Related Questions