0

Here, productIds has ID of all ordered items in an array like ["1294382", "2913892"]

So I'm trying to get all the items with those ID's from MongoDB's database. I'm mapping through each of them, and store them in orderedItems array. So that i can access price (or any property) of the item in database.

productRouter.post("/", async (req, res) => {
 
  const productIds = req.body.orderedItems.map((item) => item.id); 

  let orderedItems = [];

  productIds.map(async (productId) => {
    const d = await Product.findById(productId);
    orderedItems.push(d);
  });

  const unusedVariable = await Product.findById("61...da1"); //If i delete that line (or only await), orderedItems returns empty array. 

  console.log(orderedItems);
  
});

Now, expected output is:

[
  {
    _id: new ObjectId("614632cc8aa9513567dfbca4"),
    name: 'ürün1',
    desc: 'ürün1 açıklama',
    price: 50,
    __v: 0
  },
  {
    _id: new ObjectId("614632cc8aa9513567dfbca2"),
    name: 'kartal kupa',
    desc: 'kartal resimli kupa',
    price: 50,
    __v: 0
  }
]

And I get that output, but only if I have the unusedVariable line with await. If i remove that line, or remove "await" from there, the output i get is: [] . So an empty array instead of filled with objects.

Why ? And how do i solve that (just leave unusuedVariable there?) ? How bad is this code ?

Koray Aydemir
  • 310
  • 3
  • 9
  • 1. don't use `map` when you mean `forEach` (it's confusing) 2. a forEach loop with await in the callback doesn't run in sequence and the code doesn't wait for the promises to finish. Use `const orderedItems = await Promise.all(productIds.map(productId => Product.findById(productId)));` –  Sep 20 '21 at 17:18
  • You should instead try making an array of findById operations and pass that array to Promise.all() and await that. – Alexander Staroselsky Sep 20 '21 at 17:19
  • you are pushing in a callback, sure you made callback asynchronous but there is nothing awaiting the callback – Krzysztof Krzeszewski Sep 20 '21 at 17:20
  • Does this answer your question? [How to return the response from an asynchronous call](https://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-asynchronous-call) – Krzysztof Krzeszewski Sep 20 '21 at 17:20

1 Answers1

1

You have a concurrency issue. ​There is nothing unusual, you are not waiting for promises at (1) to end. The only reason your code is working is that MongoDB in your case uses a single connection and hence (2) is queued and completed only after (1) (all of them) are completed (it is a lucky coincidence of yours).

productRouter.post("/", async (req, res) => {
 ​const productIds = req.body.orderedItems.map((item) => item.id);
 ​let orderedItems = [];

 ​productIds.map(async (productId) => { // (1)
   ​const d = await Product.findById(productId);
   ​orderedItems.push(d);
 ​});

 ​const unusedVariable = await Product.findById("61...da1"); // (2)
 ​console.log(orderedItems);
});

As mentioned all-around nor map nor forEach supports promises (and they probably will never, due to backward compatibility) hence you should either use Promise.all which may give you some concurrence, or use plain for which is compatible with async/await syntax.

productRouter.post("/", async (req, res) => {
  const productIds = req.body.orderedItems.map((item) => item.id);
  let orderedItems = [];

  for (const productId: productIds) {
    orderedItems.push(await Product.findById(productId));
  }
  
  console.log(orderedItems);
});

Or

productRouter.post("/", async (req, res) => {
  const productIds = req.body.orderedItems.map((item) => item.id);
  
  const orderedItems = await Promise.all(
    productIds.map(productId => Product.findById(productId))
  );
  
  console.log(orderedItems);
});

Offtopic

As mentioned in the comments you should consider using $in operator, which will drastically reduce the number of queries against your DB:

productRouter.post("/", async (req, res) => {
  const productIds = req.body.orderedItems.map((item) => item.id);
  const orderedItems = Product.find({_id: {$in: productIds}});
  console.log(orderedItems);
});

More here

Newbie
  • 4,462
  • 11
  • 23
  • Thanks. That answers everything. So, when i use await in a callback function, code doesn't wait it to complete the task. Therefore returns empty orderedItems. So I just use Promise.all in such cases. Also the code you provided worked. – Koray Aydemir Sep 20 '21 at 17:41
  • Not exactly, when you use `await` you wait for a promise to end. But when you use __`async`__ you just declare a function that will return a Promise. (In your case `productIds.map(async ...` returns you an array of promises, but no one is waiting for them) – Newbie Sep 20 '21 at 17:48
  • The link mentioned above will explain you better this process: https://stackoverflow.com/questions/14220321/how-to-return-the-response-from-an-asynchronous-call – Newbie Sep 20 '21 at 17:48
  • 1
    Alright i will study that. Also about the Offtopic part, this may help someone: The code you provided doesn't work as expected, but this does (need to make them into ObjectId): `const objIds = productIds.map((id) => mongoose.Types.ObjectId(id)); const orderedItems = await Product.find({ _id: { $in: objIds }, });` I'll use that one then since its more optimized – Koray Aydemir Sep 20 '21 at 18:14