feat(blog): refresh asynchronous-flow-control.md
#7972
Conversation
…lanations The previous version of the Asynchronous Flow Control guide relied on callback-based patterns which, while historically important, led to "callback hell" and are no longer considered best practice. Changes include: * Replacing the callback-hell example with a modern async/await section to introduce the reader to a better solution. * Rewrote all core examples (In Series, Parallel, Limited Parallel) to use async/await, Promise.all, and Promise.race. * Simplified the Beer Song problem explanation while maintaining the details intact. * Corrected minor mistakes that could result in a misunderstanding. Signed-off-by: Ahmed Emad <ahmed.emad.edu@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
avivkeller
changed the title
asynchronous-flow-control.md
|
Isn't the whole point of this article the callback pattern? We have separate articles for async/await (https://nodejs.org/en/learn/asynchronous-work/javascript-asynchronous-programming-and-callbacks and https://nodejs.org/en/learn/asynchronous-work/discover-promises-in-nodejs) |
That is true, I initially thought it will be redundant but I do not see the point of writing the examples using callbacks ( noting that it is talked about here here). So why not have it in the way I wrote it? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #7972 +/- ##
==========================================
+ Coverage 73.07% 73.13% +0.05%
==========================================
Files 95 95
Lines 8358 8358
Branches 218 217 -1
==========================================
+ Hits 6108 6113 +5
+ Misses 2249 2244 -5
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Ahmed Emad <ahmed.emad.edu@gmail.com>
There was a problem hiding this comment.
I'm sorry, but I just don't think that this is something that's worth pursuing. The way I see it, we have multiple articles describing how to use async/await, and this article describes how to use callbacks. It even warns users about "callback hell" in an earlier paragraph. If you strongly feel that this article is really so bad, perhaps it should be rewritten entirely. I don't think it makes sense to introduce users to one concept (Callbacks), only to follow-up with how it's not viable.
If other @nodejs/nodejs-website members disagree, I'll rescind this block, but for the time being, it stands.
We also have another article that describes Callbacks, this article is not specific to Callbacks, for a lot of people, this article is about asynchronous flow control, which is - Like you guessed - not limited to explaining Callbacks. And since it already warns about "Callback hell" why would we show people the problem without immediately giving them the escape route? It's like showing some a broken car and then making them read another book just to find out there's a perfectly good one over there. I am not saying that Callbacks are useless, it's just that Anyway, I am happy to tweak some of the changes in this PR to make it fit it in some way, perhaps with few modifications, and some of the changes can be moved to other articles. TL;DR: Helpful PR, merge please. |
I don't think you have the right to say what we should or not merge. That said, @avivkeller it's not up to our team to say what should or not be the content here. The block is fine, but kinda contradicts the content vs code responsibility of the website team. I believe we should have @nodejs/collaborators to chime in on this regarding if this content does make sense or not. Maybe @mcollina has some opinions on this. |
|
(Note that I haven't checked the content changes here, so I'm not aware of what got changed or not, I'll give it a read later) |
Because, in my opinion, it convolutes the article, when we already have other articles explain this. If you want to show users the "escape route", that's perfectly fine, let's link to https://nodejs.org/en/learn/asynchronous-work/discover-promises-in-nodejs, but when we add new paragraphs describing something that is already described elsewhere, it increases the maintenance burden of the website.
As @ovflowd said above. We are the maintainers of the site, and have the final say on what does or does not get merged. We aren't saying your PR is not helpful, almost every PR is, however, we follow a consensus seeking model, and will therefore not merge this PR until consensus is reached.
No working group has taken ownership of this content (AFAIK we don't have a working group assigned to async/Promise behavior, so, per the responsibility, we have editorial ownership. As an aside, and this is most definitely something for a separate issue, but also worth mentioning: We, in my opinion, should be allowed to act as an editorial team for any and all content on the Node.js Website. While Core Collaborators should obviously have the final say when it comes to content they have the expertise on, the Website Team should still be able to provide editorial oversight to ensure consistency, clarity, tone, accessibility, and alignment with the broader communication standards of the project's website. It feels contradictory to say the Website Team is not editorial, while at the same time acknowledging that we write and maintain a large portion of the site’s content. If we're producing content, shouldn't we also be trusted to provide editorial oversight for that content? And if we are the default editors for unowned sections, doesn’t that imply a kind of editorial responsibility already? |
Looks like I did not clarify the joke enough. Anyhow, for the changes, I think there's a mistake with the little concurrency note, it should be removed soon, thanks for your time. |
Fair argument. It is true that we should not add a section to explain what async/await are, but instead, we could briefly demonstrate its purpose and then continue using it throughout the article, just like how the article introduced the use of callbacks (Instead of telling people "Oh yeah, there's a better solution over there, but we will continue with our current", we could just use the other one, simple :)). |
|
This should probably just be a new page. Async flow control with callbacks definitely still needs a doc explaining it as many find it confusing and, even if much code lately is async/await-based, there's still a lot of ecosystem code with callbacks. Core still uses them in many places too. The patterns need to be understood. Having content explaining the async flow control of async/await is valuable too, but not at the cost of losing documentation of how it works with callbacks. The callback page could be retitled to be specifically about callbacks and then make another using the original title or a title specific to async/await to differentiate them. |
FWIW we have a page describing Promises and async/await, but I like your proposal |
Yeah, we also have another page explaining Callbacks, this is not limited to how async/await work, but the changes use async/await. Did you read the changes? |
I like that, but would you prefer having 2 separate pages regarding this topic (Like what we have with Callbacks and Promises) or have it all in one article but with separate sections? I would like to hear your opinion so that I can start working on the changes. |
The previous version of the Asynchronous Flow Control guide relied on callback-based patterns which, while historically important, led to "callback hell" and are no longer considered best practice.
Changes include:
Replacing the callback-hell example with a modern async/await section to introduce the reader to a better solution.
Rewrote all core examples (In Series, Parallel, Limited Parallel) to use async/await, Promise.all, and Promise.race.
Simplified the Beer Song problem explanation while maintaining the details intact.
Corrected minor mistakes that could result in a misunderstanding.
Description
Changes include:
Replacing the callback-hell example with a modern async/await section to introduce the reader to a better solution.
Rewrote all core examples (In Series, Parallel, Limited Parallel) to use async/await, Promise.all, and Promise.race.
Simplified the Beer Song problem explanation while maintaining the details intact.
Corrected minor mistakes that could result in a misunderstanding.
Validation
The new text content that was added had been verified through the MDN docs.
All the added code had been tested and verified to serve the teaching purposes of the Asynchronous Flow Control documentation.
Check List
pnpm formatto ensure the code follows the style guide.pnpm testto check if all tests are passing.pnpm buildto check if the website builds without errors.