doc: add esm examples to node:readline#55335
Conversation
nodejs-github-bot
added
doc
| ``` | ||
|
|
||
| ```cjs | ||
| const readlinePromises = require('node:readline/promises'); |
There was a problem hiding this comment.
If you use destructuring in the ESM, it should probably also be used here.
There was a problem hiding this comment.
Ok! I generally try not to touch the examples that are already there, but if you think that'd be better I'll do it!
Should I do it for that example in particular or for the rest of the examples as well?
Thanks for the feedback! 🙌
There was a problem hiding this comment.
| const { stdin: input, stdout: output } = require('node:process'); | ||
| const rl = createInterface({ input, output }); |
There was a problem hiding this comment.
This example and the MJS example should be as close to identical as possible, except for the import.
However, you don't need to require process in CJS
There was a problem hiding this comment.
In other words, the examples should look exactly the same (except the import + process). That way, users can ensure consistency
There was a problem hiding this comment.
Done!
Thank you for taking the time to review the changes and give feedback to me 🙏 !
|
hello @redyetidev I've made the changes you requested, can I kindly ask you to review once more and if everything looks good to you you can approve the PR? 😊 if not, let me know what other changes you'd like me to make! I generally try to not change everything too much just so there's a lot of consistency between the Thanks in advance! 🙌 |
lpinca
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in a7c9f85 |
PR-URL: #55335 Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
4 participants
This PR adds the missing
ESMcounterparts of theCJSexamples for the Readline documentation.I've tested every single example and they all work/behave as expected.
Best regards!