-2

I'm just trying to understand the benefits of this:

const populateUsers = done => {
  User.remove({}).then(async () => {
    const userOne = new User(users[0]).save();
    const userTwo = new User(users[1]).save();
    const usersProm = await Promise.all([userOne, userTwo]).then(() => done());
    return usersProm;
  });
};

over this:

const populateUsers = done => {
  User.remove({})
    .then(() => {
      const userOne = new User(users[0]).save();
      const userTwo = new User(users[1]).save();

      return Promise.all([userOne, userTwo]);
    })
    .then(() => done());
};

I came to this problem because eslint suggested my to use async in this function, and I remember the concept, make it work in my app, but I'm not sure why should I use this instead of the original way

Heretic Monkey
  • 11,687
  • 7
  • 53
  • 122
Daniel C
  • 81
  • 1
  • 1
  • 7

2 Answers2

0

Your first version does not go all the way. Do this:

const populateUsers = done => {
  User.remove({}).then(async () => {
    const userOne = new User(users[0]).save();
    const userTwo = new User(users[1]).save();
    await Promise.all([userOne, userTwo]);
    const usersProm = await done();
    return usersProm;
  });
};

There is no difference, it is just that code without these then callbacks is somewhat easier to read.

You might even apply it to the outer function:

const populateUsers = async () => {
  await User.remove({});
  const userOne = new User(users[0]).save();
  const userTwo = new User(users[1]).save();
  await Promise.all([userOne, userTwo]);
  const usersProm = await done();
  return usersProm;
};

Now populateUsers returns the promise instead of undefined.

As concluded in comments: you get an error because populateUsersreturns a promise and accepts a done callback argument, while one of these is expected, not both.

trincot
  • 317,000
  • 35
  • 244
  • 286
  • In order to be sure, when I use await, I do not need use then() because only until the await promise "ends" my code will not continue, then I just have to call the done(). I modified your code in order to understand. Would this work too, or it will give me some problems in the future? const populateUsers = done => { User.remove({}).then(async () => { const userOne = new User(users[0]).save(); const userTwo = new User(users[1]).save(); await Promise.all([userOne, userTwo]); return done(); }); }; – Daniel C Feb 26 '19 at 21:46
  • When `await` is encountered, the `async` function returns immediately with a promise. Execution continues there as usual. When the call stack is empty and the promise at the `await` resolves, then magically the function's context is restored, and continues there. Yes, your variant is fine too. – trincot Feb 26 '19 at 21:53
  • Ok, I got it with the call stack. But, when I used the last function with the await User.remove({}), It didn't pass the test. " Error: Resolution method is overspecified. Specify a callback *or* return a Promise; not both." Why this is not working? – Daniel C Feb 26 '19 at 22:00
  • I guess you get that error in the way you call `populateUsers`. The context where you call it will expect a returned promise or the `done` argument, not both. – trincot Feb 26 '19 at 22:07
  • I deleted the done callback and now everything works as inteded, thanks – Daniel C Feb 26 '19 at 22:09
  • @trincot Note, no value is `return`ed from the function at the code at the first example at the answer. A value is returned from within `.then()`, though not from the `populateUsers` function, that is, `return User.remove({}).then()` – guest271314 Feb 27 '19 at 03:16
  • @guest271314, I assume this comment is for the OP, as this is the behaviour presented in the question. – trincot Feb 27 '19 at 06:07
  • @trincot The answer states "Do this:". Not sure if it is clear to OP why the function would result in `undefined`, or an error if built-in `.then()` is chained. Even though the question is about returning a `Promise`, no value is returned from the first example code at the question. Though you did point out _"Now `populateUsers` returns the promise instead of `undefined`."_ – guest271314 Feb 27 '19 at 06:17
0

Your original code was totally fine.

No, there is no benefit in using the code from your first snippet. You should avoid mixing await and .then(…) syntax! To use async/await, you'd make the whole function async, not the then callback:

async function populateUsers(done) {
  await User.remove({})
  const userOne = new User(users[0]).save();
  const userTwo = new User(users[1]).save();

  await Promise.all([userOne, userTwo]);
  return done();
}

(Probably you would also remove that done callback - the function already returns a promise)

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • 1
    Ok, with your answer and the one from Trincot, I understood exactly what does the async functions with the await, and why I got an error like this "Error: Resolution method is overspecified. Specify a callback *or* return a Promise; not both." I removed the done callback and everything worked as inteded. thanks – Daniel C Feb 26 '19 at 22:08