Why is the empty array still empty even if I push values in it?

Question

usersList is a valid array with string values, User.findById(userId) returns an user as expected. I think the problem lies in the block of scope because forEach method and FindById method works perfect.

I've tried many methods of array returning each value. (e.g. map and filter) I've read many documents about advanced MongoDB technique, the block of scope and array methods.

const usersList = camp.usersList;
let users = [];
usersList.forEach((userId) => {
  User.findById(userId, (err, user) => {
    if(err) return res.redirect('/admin/db');
    users.push(user);
  })
})
console.log(users);

The output should be [user1, user2, ...], not [].

Befote pushing `user` to `users` array please print the value on console (`console.log('User is '+user);`). The problem might be the push was never called. if there was no user in database. Secondly, the time you are printing `users` database query was not finished (async). — fiveelements, Aug 09 '19 at 16:02
Is User.findById asynchronous? forEach ignores async functionality; in addition to not using async/await with your function, you'd hit `console.log(users)` before your function made the `users.push(user)` call regardless. — Epoch, Aug 09 '19 at 16:05
Thank you for your answers, async/await solved the problem for me! — , Aug 09 '19 at 16:16

Nayan Patel · Accepted Answer · 2019-08-09T17:11:14.440

2

async () => {
    const usersList = camp.usersList;
    const users = [];
    for (var userId of usersList)  {
        const user = await User.findById(userId).exec();
        if (!user) return res.redirect('/admin/db');
        users.push(user);
    }
    console.log(users);
}

For better Performance, you can use this below code, it will not wait for individual request to complete, it will make parallel calls and hence increase the performance

async ()=> {
    try {
        const users_promise = usersList.map(userid => User.findById(userid).exec())
        const users = await Promise.all(users_promise);
    } catch(error) {
        res.redirect('/admin/db');
    }
}

edited Aug 09 '19 at 17:11

answered Aug 09 '19 at 16:03

Nayan Patel

1,683
25
27

Should be noted that the reason Nayan changed the loop from `.forEach` is because `.forEach` doesn't support async. – Epoch Aug 09 '19 at 16:06
forEach uses callbacks, so we should not use await there. So whenever we need to await for the response. you should not use forEach. – Nayan Patel Aug 09 '19 at 16:09
1

As far as I can tell, Mongoose's [`Model.findById`](https://mongoosejs.com/docs/api.html#model_Model.findById) returns a [`Query`](https://mongoosejs.com/docs/api/query.html), not a promise. You'll need to call [`query.exec()`](https://mongoosejs.com/docs/api/query.html#query_Query-exec) I believe. – Khauri Aug 09 '19 at 16:09
2

@Khauri You're correct, but the result is still thenable (so it can be used with await). https://mongoosejs.com/docs/queries.html#queries-are-not-promises `User.findById(userId).then` is basically a shortcut for `User.findById(userId).exec().then`. – Paul Aug 09 '19 at 16:11
Javascript is amazing – Khauri Aug 09 '19 at 16:16
@Khauri you were correct , i made a mistake, i corrected it now. Thanks for the suggestion. – Nayan Patel Aug 09 '19 at 16:17

score -1 · Answer 2 · answered Aug 09 '19 at 16:07

-1

Use array of promises instead

const userPromises = usersList.map(userid => User.findById(userid))

Promise.all(userPromises).then(users => console.log(users))
                         .catch(() => res.redirect('/admin/db'));

answered Aug 09 '19 at 16:07

charlietfl

170,828
13
121
150

Why is the empty array still empty even if I push values in it?

2 Answers2