You can just use regular promises, a regular for
loop and async/await
like this. There is no need for an external library function such as async.series()
here:
let lor = await Articles.find({ Status: 1, isPublish: true})
.sort({TotalClapCount: -1})
.sort({ViewCount: -1})
.skip(offset)
.limit(limit);
for (let data of lor) {
let hre = await ArticleBookmarks.find({
ArticleID: data.ID,
UserID: ArgsUserID,
Status: 1
}).countDocuments();
data.isBookmark = (hre == 1);
}
return lor;
Changes:
There's no use in return await Articles.find()
as it does nothing different than return Articles.find()
since both just return a promise since it's inside an async
function and all async
functions return a promise.
Change .forEach()
to a regular for
loop because a for
loop WILL pause execution of the function for await
. forEach()
will not pause the loop.
await xxx.forEach()
does not await
anything because .forEach()
has no return value and await
only does something useful when you await
a promise.
Change (hre == 1) ? true : false;
to just (hre == 1)
because hre == 1
is already a boolean so there's no need for ? true : false
. Personally, I would probably use (hre === 1)
if you know the data is actually a number and not a string since ===
is nearly always more desirable than ==
(no implicit or accidental type conversions).
If you're going to use await
, then it's usually simpler to use it other places in that function instead of .then()
as it leads to more sequential looking code that most would say looks simpler.
Other comments:
await
should only be used when you're awaiting a promise. That's the only time is does something useful.
Wrapping new Promise()
around other promise returning functions is considered a promise anti-pattern as it is not necessary and creates many opportunities for programming mistakes, particularly with error handling. You can just return and use the promises you already have without wrapping in a new one.
async.eachSeries()
offers nothing that regular promise sequencing using async/await
can already do just fine unless you're trying to run in an environment that doesn't have async/await
.
When you want await
to pause a loop, use a regular for
loop. It does not pause the loop for array methods such as .map()
or .forEach()
because those iteration methods are not promise-aware. They don't pause on a promise returned from their callback (which is what happens when you declared the callback as async
).
Another Possibility with Parallel Queries
Since the processing of one iteration of the loop is independent from the others, you could run all those database queries in parallel and use Promise.all()
to know when they are done. That has one downside that if the array is large, you will be throwing a lot of queries at the database all at once. The upside is that the end-to-end processing time may be less if the database is not overwhelmed by the number of simultaneous queries. Here's how you could do that:
let lor = await Articles.find({ Status: 1, isPublish: true})
.sort({TotalClapCount: -1})
.sort({ViewCount: -1})
.skip(offset)
.limit(limit);
await Promise.all(lor.map(async data => {
let hre = await ArticleBookmarks.find({
ArticleID: data.ID,
UserID: ArgsUserID,
Status: 1
}).countDocuments();
data.isBookmark = (hre == 1);
});
return lor;