nest-cli
nest-cli copied to clipboard
feat: add flag to disable shell mode for `nest start --watch`
When running on Linux/WSL, nest start --watch
does not wait the app to shutdown on restart. That leads to two instances of application running simultaneously, especially if it has async shutdown hooks. That commit fixes that adding an option to disable shell mode when spawning child processes so then app will completely shutdown first and only then started again. Thanks @jleverenz for the investigation and solution idea in #1641
fixes nestjs/nest-cli/1614
PR Checklist
Please check if your PR fulfills the following requirements:
- [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:
What is the current behavior?
Nest does not wait for app to stop when doing hot reload (on files change)
Issue Number: #1614
What is the new behavior?
Now running nest start --watch --noShell
hot reload will first wait for app to gracefully stop
Does this PR introduce a breaking change?
[ ] Yes
[x] No
Other information
Any update on this? 🙂 The EADDRINUSE issue makes local development rather inconvenient
@kamilmysliwiec is there smth we can do to get this PR merged?
I'm not sure about the name 'no shell'. To me, it doesn't tell what will happen when using it. Do you know any other high level name? --noShell
sound like we're exposing a detail implementation of @nestjs/cli
. So if we want to change that implementation later, the flag will be out of sync somewhat
But I can't think of any other name tho
To be fair, I'm not sure if we should have shell: true
I'm not sure about the name 'no shell'
I agree that the name isn't self-descriptive (though there is a description for that flag).
We can probably use smth like --shell=false
since spawn()
's shell
arg supports different shells and we can either provide a custom shell (for whatever reason, just to be less specific then noShell
) or disable it.
To be fair, I'm not sure if we should have
shell: true
Me neither, i decided to introduce a flag just in favor of backward compatibility
Thanks for this PR. Any update this? This will improve my DX.