-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
feat(core): add options parameter to repl #12924
feat(core): add options parameter to repl #12924
Conversation
Enables passing node repl options parameter to nestjs/core/repl fixes nestjs#12916
Pull Request Test Coverage Report for Build 8e7cb4c5-2c2a-40ff-bd50-32f96d6b3646
💛 - Coveralls |
@@ -21,6 +26,7 @@ export async function repl(module: Type | DynamicModule) { | |||
const replServer = _repl.start({ | |||
prompt: clc.green('> '), | |||
ignoreUndefined: true, | |||
...replOptions, |
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.
so now users can change the prompt
. That's good, to me
But I wonder if changing the ignoreUndefined
will break the repl 🤔 I don't why we're using ignoreUndefined: true
tbh
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.
I wondered the same thing. Not really sure why ignoreUndefined
is set to true
but there's probably a reason behind ?
That's how it behaves :
- node repl
> undefined
undefined
- nestjs repl
> undefined
>
It's actually weird haha.
If we can confirm this is required, I can move the replOptions
spread at the begining but that would prevent users to change the promp. I clould also force ignoreUndefined: true
but keep the rest of replOptions
. (and make it clear for users in typing like : Omit<ReplOptions, 'ignoreUndefined'>
)
wdyt ?
lgtm |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The
nestjs/core/repl
function that runsnode:repl
does not allow the consumers to providenode:repl
's option (ReplOptions
).Issue Number: #12916
What is the new behavior?
This PR allows passing
ReplOptions
to thenestjs/core/repl
function. Current implemented behavior will also override default optionsprompt
/ignoreUndefined
if they are provided. If required, I can change the PR to not override this default options.Does this PR introduce a breaking change?
Other information