CODEWITHSUNDEEP
javascriptmongoosepromisees6-promise
ecwalsh
ecwalsh

Reputation: 63

Promise resolves before forEach loop completes

I'm building my first CRUD (library) application and recently learned about promises as a means to avoid deeply-nested callbacks. I'm attempting to seed my DB with some data each time the server starts, but I seem to be missing something conceptually.

I have four objects in a bookData array that I want to iterate over and save to the DB using Mongoose:

function seedBooks() {
    return new Promise(function(resolve,reject){
        bookData.forEach(function(seed){
            Book.create(seed, function(err, newBook){
                if(err) {
                    reject(err);
                }
            });
        });
        resolve();
    });
}

This function is one of a few that I am attempting to chain together, which is why I'm using promises. But I'm finding that seedBooks() resolves with anywhere between 1 and 4 of the sample books created, 

function seedDB() {
    removeAllBooks()
    .then(function(){
        return removeAllUsers();
    })
    .then(function(){
        return seedUsers();
    })
    .then(function(){
        return seedBooks();
    })
    .then(function(){
        return seedBookInstances();
    });
}

Am I understanding or using promises & resolve incorrectly? Any help is appreciated. Thanks!

Upvotes: 2

Views: 825

Answers (3)

JLRishe
JLRishe

Reputation: 101690

Edit: The below explains why your code is not working and advice for, in general, how to convert non-promise code to promises. Since Mongoose produces promises, however, you should be using those instead of using new Promise. See Olegzandr's answer regarding that.

The promise is resolving immediately because you are calling resolve() immediately.

The rule of thumb when converting non-promises to promises is to promisify the smallest part of the non-promise code. In this case, it means promisifying the code to save a single item. If you do that, the collect place to call resolve() becomes clear:

function seedBook(seed) {
    return new Promise(function (resolve, reject) {
        Book.create(seed, function (err, newBook) {
            if (err) { reject(err); } else { resolve(newBook); }
        });
    });
}

function seedBooks() {
    return Promise.all(bookData.map(seedBook));
}

This also has the benefit of allowing you to access the returned newBooks, should you want to.

Upvotes: 2

user5047085
user5047085

Reputation:

If you are using Mongoose, you can just do this:

const saveBooks = function(books) {
    return books.map(function(seed) {
            return Book.create(seed);  // returns a promise
        });
    });
}

return Promise.all(saveBooks(books)).then(function(){
    // all books are saved
});

Upvotes: 1

dhilt
dhilt

Reputation: 20744

You are resolving your Promise synchronously, right after you started your forEached requests, which are async. You may try the following approach instead:

function seedBooks() {
    return new Promise(function(resolve,reject){
        var count = 0, length = bookData.length;
        bookData.forEach(function(seed){
            Book.create(seed, function(err, newBook){
                if(err) {
                    reject(err);
                    return;
                }
                if(count++ >= length ) {
                  resolve();
                }
            });
        });
    });
}

Here the Promise is being resolved only after all async requests are done.

Another option would be just to use Promise.all. In that approach you need to promisify all your requests in the loop, return an array of Promises and then call Promise.all(_seedBooks()).then(), where _seedBook returns an array of Promises:

function _seedBooks() {
    return bookData.map(function(seed) {
        return new Promise(function(resolve, reject) {
            Book.create(seed, function(err, newBook) {
                if(err) {
                    reject(err);
                    return;
                }
                resolve(newBook);
            });
        });
    });
}

Promise.all(_seedBooks())
.then(function(result) { /* result is the array of newBook objects */ })
.catch(function(error) { /* error is the first rejected err */ })

Upvotes: 0

Related Questions