-
-
Notifications
You must be signed in to change notification settings - Fork 72
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: quiet flag only prints when set to false #60
Conversation
1 similar comment
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.
@@ -26,8 +26,8 @@ module.exports = function runCompilation (options, compiler, done) { | |||
return file.errored; | |||
}); | |||
|
|||
if (!options.quiet) { | |||
console.log(chalk.yellow(options.formatter(results))); | |||
if (options.quiet === false) { |
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.
Hey @JaKXz, I tested this locally.
It seems like this change has no effect, because quite
is set to false by default here.
Wonder what is the reason of using quite
here if webpack outputs warnings and errors to the console as well in onCompilation
?
Maybe we could move it to onCompilation
and have something like this:
compiler.plugin('after-emit', function onCompilation (compilation, callback) {
if (warnings.length) {
if (options.quiet === false) {
compilation.warnings.push(chalk.yellow(options.formatter(warnings)));
}
warnings = [];
}
if (errors.length) {
if (options.quiet === false) {
compilation.errors.push(chalk.red(options.formatter(errors)));
}
errors = [];
}
callback();
});
If at least something should be in the output, it can be:
if (warnings.length) {
var formattedWarnings = chalk.yellow(options.formatter(warnings));
if (options.quiet === false) {
compilation.warnings.push(formattedWarnings);
} else {
console.log(formattedWarnings);
}
warnings = [];
}
what do you think?
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 removed the default quiet value in this change though. I'll take a closer look at your suggestion later, but could you try installing this branch locally and I think it will solve the problem.
1 similar comment
@sergesemashko I'm going to merge this for now so we can test |
Looks good, duplicates are gone now |
* refactor: pull out errorMessage constant
No description provided.