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

Fix https://github.com/nestjs/nest-cli/issues/1995 #1997

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hkjeffchan
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?
Bug fix

[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?

webpack cache is not created using nest build if cache is set in webpack.config.js

Issue Number: 1995

What is the new behavior?

webpack cache is created using nest build if cache is set in webpack.config.js

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@hkjeffchan
Copy link
Author

Updated the PR.

@@ -140,6 +140,9 @@ export class WebpackCompiler {
return process.exit(1);
}
console.log(statsOutput);
if (!watchMode && !watch) {
compiler.close(() => {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we move this up - above the process.exit()?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it won't run... we need to move inside "!err && !stats.hasErrors()"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just leave it here at line 143

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

      if (!err && !stats!.hasErrors()) {
        if (!onSuccess) {
          assetsManager.closeWatchers();
        } else {
          onSuccess();
        }
      } else if (!watchMode && !watch) {
        console.log(statsOutput);
        return process.exit(1);
      }
      console.log(statsOutput);
      if (!watchMode && !watch) {
        compiler.close(() => {});
      }

or

      if (!err && !stats!.hasErrors()) {
        if (!onSuccess) {
          assetsManager.closeWatchers();
        } else {
          onSuccess();
        }
        if (!watchMode && !watch) {
          compiler.close(() => {});
        }
      } else if (!watchMode && !watch) {
        console.log(statsOutput);
        return process.exit(1);
      }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kamilmysliwiec Would you check if you can merge this now?

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