nest-cli icon indicating copy to clipboard operation
nest-cli copied to clipboard

Fix hanging assets watchers

Open jleverenz opened this issue 3 years ago • 2 comments

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

[x] Bugfix
[ ] 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?

When starting an unwatched webpack compile on a project with assets (e.g. with nest start) the process never exits. nest build works as expected.

What is the new behavior?

It exits as expected.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

When starting an app with webpack, the closeWatchers() function is not called like it is with the tsc compiler. This turned up for me once I switched to a monorepo project, but can be reproduced on a standard project by setting "webpack": true in the compiler configuration along with assets.

While digging into this I also noticed a difference in the way that the webpack compiler is called for the "build" vs "start" action. For "start", an onSuccess callback is provided (which ultimately starts the application). For "build", no callback is provided. If no callback is provided the WebpackCompiler calls closeWatchers() on AssetsManager (presumably, the code is assuming it's in "build" mode). See #609

The Compiler and WatchCompiler don't have any references to AssetsManager.

This PR removes the functionality from WebpackCompiler and centralizes the behavior in the build action.

jleverenz avatar Feb 08 '23 01:02 jleverenz

Repro project: https://github.com/jleverenz/nestjs-assets-hang-repro

jleverenz avatar Feb 08 '23 01:02 jleverenz

I believe this PR https://github.com/nestjs/nest-cli/pull/2293 should fix this issue

kamilmysliwiec avatar Oct 23 '23 13:10 kamilmysliwiec