Skip to content
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

Use node watch mode when watch is enabled to restart the application when dependencies change #2659

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hartbit
Copy link

@hartbit hartbit commented Jul 19, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

Couldn't find any existing tests for the start action.

  • Docs have been added / updated (for bug fixes / features)

Couldn't find any existing tests for the start action.

PR Type

What kind of change does this PR introduce?

[X] 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:

This is in-between a bugfix and a new feature: I was expecting the watch mode of Nest.js to behave like nodemon or Node.js' watch mode and watch all imported files, and not like TypeScript's build watch that only watches files to build.

What is the current behavior?

Currently, the start command's watch mode only watches for files to build. But once the application has started, changes to dependencies don't trigger a restart. It's a build watch, not a run watch.

What is the new behavior?

This change appends the --watch option to the node command run when the build succeeds to also add this run watch behaviour.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

@micalevisk
Copy link
Member

It's a build watch, not a run watch.

yeah. Not sure if we really want to switch to a run watch, tho

it could be resource consuming. In the build watch approach we can optimize this

@hartbit
Copy link
Author

hartbit commented Oct 8, 2024

@micalevisk Any news on potentially merging this ? How can I resolve your worries to move this forward ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants