-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
learn(promises): update fs/promises example
#7763
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
Conversation
Signed-off-by: Vishal Kumar Gupta <groupsvkg@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this 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 adjusts the code example in the Discover Promises guide to import the full fs module and use its promises property, rather than requiring the promises API directly.
- Swapped
require('node:fs').promisesforrequire('node:fs') - Updated the
readFilecall to usefs.promises.readFile
Comments suppressed due to low confidence (1)
apps/site/pages/en/learn/asynchronous-work/discover-promises-in-nodejs.md:162
- Consider adding a note about Node.js version requirements for the direct
require('node:fs/promises')import, as it’s only available in Node.js 14+.
const fs = require('node:fs');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this better? It's the exact same thing, except the previous example worked interchangeably with the code comment, and this would break if you replace the imported version with the promise version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it's cool and more readable
|
I think we should update the text to match the example. This example is purposely formatted in such a way where that time can be replaced with an |
Signed-off-by: Vishal Kumar Gupta <groupsvkg@gmail.com>
Yes it makes sense. |
fs/promises example
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 #7763 +/- ##
==========================================
- Coverage 75.10% 75.08% -0.03%
==========================================
Files 98 98
Lines 7914 7914
Branches 196 196
==========================================
- Hits 5944 5942 -2
- Misses 1969 1971 +2
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Lighthouse Results
|

Description
Updated
[Discover Promises in Node.js](https://nodejs.org/en/learn/asynchronous-work/discover-promises-in-nodejs#discover-promises-in-nodejs)pageValidation
Related Issues
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.