Skip to content

feat(learn): add article on promises #7645

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(learn): add article on promises #7645

wants to merge 7 commits into from

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented Apr 12, 2025 •
edited
Loading

Description

This PR adds an explanation of JavaScript / Node.js Promises to the website.

I took some feedback from #7639, and tried to incorporate specific examples and real-world scenarios.

Related Issues

Fixes: #7200

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 bot review requested due to automatic review settings April 12, 2025 00:15
@avivkeller avivkeller requested a review from a team as a code owner April 12, 2025 00:15
@vercel Vercel
Copy link

vercel bot commented Apr 12, 2025 •
edited
Loading

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

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Apr 12, 2025 8:55pm

Copilot
Copilot AI reviewed Apr 12, 2025
View reviewed changes
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.

Copilot reviewed 1 out of 3 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • apps/site/navigation.json: Language not supported
  • packages/i18n/locales/en.json: Language not supported

@avivkeller
Copy link
Member Author

avivkeller commented Apr 12, 2025 •
edited
Loading

See https://nodejs-org-git-feat-learnpromises-openjs.vercel.app/en/learn/asynchronous-work/discover-promises-in-nodejs

@avivkeller avivkeller mentioned this pull request Apr 12, 2025
Open
@avivkeller avivkeller added content Issues/pr concerning content learn Issues/pr concerning the learn section labels Apr 12, 2025
mcollina
mcollina previously requested changes Apr 12, 2025
View reviewed changes
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.

This should also cover the relationship between promises, queueMicrotask, process.nextTick and setImmediate.

@avivkeller
Copy link
Member Author

@mcollina I've attempted to resolve your reviews in 8180120

@avivkeller avivkeller linked an issue Apr 12, 2025 that may be closed by this pull request
Add a Learn section about Promises/queueMicrotask and promises #7200
Open
@avivkeller avivkeller mentioned this pull request Apr 12, 2025
Closed
AugustinMauroy
AugustinMauroy reviewed Apr 12, 2025
View reviewed changes
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

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

Nice but nothing about await on top level

@avivkeller avivkeller requested review from jasnell and mcollina April 13, 2025 14:58
jasnell
jasnell approved these changes Apr 13, 2025
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

Let's ask @mcollina to review as well.

@avivkeller avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Apr 14, 2025
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Apr 14, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 14, 2025 •
edited
Loading

Lighthouse Results

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

@github-actions GitHub Actions
Copy link
Contributor

Unit Test Coverage Report

Title Lines Statements Branches Functions
@node-core/ui-components Coverage: 95%
 95.83% (161/168) 77.86% (102/131) 88.57% (31/35)
@nodejs/website Coverage: 87%
 84.7% (493/582) 76.03% (165/217) 86.88% (106/122)
Title Tests Skipped Failures Errors Time
@node-core/ui-components 24 0 💤 0 ❌ 0 🔥 4.744s ⏱️
@nodejs/website 156 0 💤 0 ❌ 0 🔥 6.603s ⏱️

@avivkeller avivkeller dismissed mcollina’s stale review April 15, 2025 15:25

All concerns were addressed, @mcollina, If are still blocking, please leave a new review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AugustinMauroy AugustinMauroy AugustinMauroy left review comments

Copilot code review Copilot Copilot left review comments

@jasnell jasnell jasnell approved these changes

@mcollina mcollina Awaiting requested review from mcollina

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
content Issues/pr concerning content learn Issues/pr concerning the learn section
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a Learn section about Promises/queueMicrotask and promises
4 participants
@avivkeller @mcollina @jasnell @AugustinMauroy