Repository navigation
doc: add esm examples to node:repl - #55432
Conversation
nodejs-github-bot
added
doc
| ```mjs | ||
| import repl from 'node:repl'; | ||
| import { Translator } from 'translator'; | ||
|
|
||
| const myTranslator = new Translator('en', 'fr'); | ||
|
|
||
| function myEval(cmd, context, filename, callback) { | ||
| callback(null, myTranslator.translate(cmd)); | ||
| } | ||
|
|
||
| repl.start({ prompt: '> ', eval: myEval }); | ||
| ``` |
There was a problem hiding this comment.
this code block adds a mock dependency to a module that actually exists and is very dormant (translator). IMO we should try to avoid that and the alternative of using a module that doesn't exist.
While this is also true of the previously existing example, since it's being updated now with a totally new example I think it's a good time to adjust it.
There was a problem hiding this comment.
Agreed! I also thought about that example since it throws an error when you try to execute it as is and new users can be frustrated by that, I'll come up with a good alternative and update as soon as I can! Thanks for the feedback! 🙌
There was a problem hiding this comment.
I've updated the example, tell me what you think! 😊 And again, thank you for taking the time to provide feedback! 🙏
| ```mjs | ||
| import repl from 'node:repl'; | ||
|
|
||
| function initializeContext(context) { | ||
| context.m = 'test'; | ||
| } | ||
|
|
||
| const r = repl.start({ prompt: '> ' }); | ||
| initializeContext(r.context); | ||
|
|
||
| r.on('reset', initializeContext); | ||
| ``` |
There was a problem hiding this comment.
I don't think this one is working as it's described? Maybe I'm misunderstanding what's supposed to happen here?
There was a problem hiding this comment.
It resets the context, m equals to 'test', but if you were to do something like m = 'other thing' and then you call .clear it will reset the context, so when you print the value of m it would be reset back to 'test' 😊
full example:
$ ./node index.js
> m
'test'
> m = 1
1
> m
1
> .clear
Clearing context...
> m
'test'
>
| help: 'Say hello', | ||
| action(name) { | ||
| this.clearBufferedCommand(); | ||
| console.log(`Hello, ${name}!`); |
There was a problem hiding this comment.
this output doesn't quite make sense - where would ${name} ever come from?
There was a problem hiding this comment.
it's passed when you execute the .sayhello command, like so:
> .sayhello bnb
Hello, bnb!
left a comment
•
There was a problem hiding this comment.
These are all really good additions! I'd like to see the comments I left addressed - specifically, for ${name} and the r.on('reset', initializeContext); example, I'd like to make sure we improve them to be fully understandable/usable examples, if possible.
commented
Dec 5, 2024
|
hello @bnb 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
commented
Dec 14, 2024
|
Landed in 2d4e35c |
This PR adds the ESM counterparts of the CJS examples for the REPL documentation.
I've tested every single example and they all work/behave as expected.
Best regards!