Skip to content

fix(learn): improve setImmediate in Promises article #7677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

pakobarbakadze
Copy link

@pakobarbakadze pakobarbakadze commented Apr 23, 2025

Description

This PR updates the documentation for setImmediate() to provide a more accurate and precise explanation of its behavior within the Node.js event loop. The new description clarifies that setImmediate() callbacks run during the check phase, immediately after I/O callbacks (poll phase), and helps distinguish its timing relative to other asynchronous mechanisms like setTimeout().

It also refines the usage guidance in the "When to Use Each" section, removing redundant and potentially confusing statements.

Validation

The example code has been reviewed to correctly reflect the behavior of setImmediate() in relation to I/O and timers.

Reviewers should verify that the updated explanation is both technically correct and clearly communicates when and why to use setImmediate(). No functional or visual changes—only documentation improvements.

Related Issues

Fixes: nodejs/node#57993

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run format to ensure the code follows the style guide.
  • I have run npm run test to check if all tests are passing.
  • I have run npx turbo build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

@Copilot Copilot AI review requested due to automatic review settings April 23, 2025 17:44
@pakobarbakadze pakobarbakadze requested a review from a team as a code owner April 23, 2025 17:44
Copy link

vercel bot commented Apr 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview May 19, 2025 0:32am

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the documentation for setImmediate() by clarifying its behavior in the Node.js event loop and refining usage guidance.

  • Updated the explanation for setImmediate() to specify that its callbacks are scheduled in the event loop’s check phase after I/O callbacks.
  • Augmented the code example to include an I/O operation and a timer to illustrate the order of execution.
  • Revised the "When to Use Each" section to provide clearer guidance on the use of setImmediate().

@codecov-commenter
Copy link

codecov-commenter commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.05%. Comparing base (0b24039) to head (ba7eec6).
Report is 56 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7677      +/-   ##
==========================================
+ Coverage   74.63%   75.05%   +0.41%     
==========================================
  Files          96       98       +2     
  Lines        7689     7914     +225     
  Branches      192      196       +4     
==========================================
+ Hits         5739     5940     +201     
- Misses       1948     1973      +25     
+ Partials        2        1       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@avivkeller avivkeller changed the title Fix setImmediate docs fix(learn): improve setImmediate in Promises article Apr 23, 2025
@pakobarbakadze
Copy link
Author

Hey @avivkeller — I noticed that the approval didn't trigger the checks. Is there anything else I should do to get them running?

@avivkeller
Copy link
Member

avivkeller commented Apr 23, 2025

Hi, @pakobarbakadze. The checks are run manually, so no need to re-request approval.

An approving review is different than a workflow run approval.

@avivkeller avivkeller added content Issues/pr concerning content github_actions:pull-request Trigger Pull Request Checks learn Issues/pr concerning the learn section labels Apr 23, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Apr 23, 2025
Copy link
Contributor

github-actions bot commented Apr 23, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 99 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about/previous-releases 🟢 99 🟢 100 🟢 100 🟢 92 🔗
/en/download 🟢 98 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 100 🟢 100 🟢 96 🟢 92 🔗

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the proposed documentation is any better.

@pakobarbakadze
Copy link
Author

@ovflowd Initially, I suggested using a setImmediate example related to setTimeout and I/O to illustrate its role in the event loop. However, since this blog targets beginners, me and @avivkeller decided that that approach may have been too complex. While the current proposal doesn’t fully clarify where setImmediate fits in the event loop, it does help reduce confusion around timers—which is a good step forward.

Could you please suggest how we might improve the explanation further?

Thanks.

@avivkeller
Copy link
Member

@ovflowd IIUC (and I might be wrong), @pakobarbakadze pointed out that the specific "but before timers" claim may not be accurate, so it was removed.

If I misunderstood, LMK.

@avivkeller avivkeller requested a review from ovflowd May 12, 2025 14:16
@ovflowd
Copy link
Member

ovflowd commented May 19, 2025

Im fine removing my block, but @avivkeller, we don't own content. Please keep that in mind, I'd like to have @nodejs/collaborators chiming in to say if this change is accurate and reflects their perview.

@Trott
Copy link
Member

Trott commented May 19, 2025

@nodejs/timers

@jakecastelli
Copy link
Member

jakecastelli commented May 19, 2025

The event loop in Node.js is provided by libuv. If we dive into the source code — specifically this line in core.c — we can see that the check phase is scheduled after the io_poll phase. Interestingly, there’s also another run_pending phase, which can run up to 8 pending callbacks. This phase isn’t mentioned in the graph. And there is due timer phase in the end.

For reference, libuv’s official event loop design is described here: libuv Design Overview.

Since this is part of a learning-focused article, I am not entirely sure what would be better wording.

@ovflowd
Copy link
Member

ovflowd commented May 19, 2025

The event loop in Node.js is provided by libuv. If we dive into the source code — specifically this line in core.c — we can see that the check phase is scheduled after the io_poll phase. Interestingly, there’s also another run_pending phase, which can run up to 8 pending callbacks. This phase isn’t mentioned in the graph.

For reference, libuv’s official event loop design is described here: libuv Design Overview.

Since this is part of a learning-focused article, I am not entirely sure what would be better wording.

I believe my ask is if the proposed changes and revised document are in-line with real-world (and/or technical expectations) for setImmediate.

@jakecastelli
Copy link
Member

@ovflowd Sorry if I've brought in more confusion!

If we consider

I believe my ask is if the proposed changes and revised document are in-line with real-world (and/or technical expectations) for setImmediate.

This change looks fine to me, it simplified the check phase in the event loop where setImmediate is run.

@jakecastelli
Copy link
Member

"but before timers" claim may not be accurate, so it was removed.

This should be before due timers, but considering this is a learn article, maybe simplifying it makes more sense. @avivkeller (in my opinion though)

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content Issues/pr concerning content learn Issues/pr concerning the learn section
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs Incorrectly State setImmediate() Runs Before Timers
7 participants