Skip to content

Commit 6ded275

Browse files
evenstensbergevenstensberg
committedFeb 5, 2019
fix: close compiler, own sh script and output clearing
·
webpack-cli@6.0.13.3.3
1 parent 2ed8c60 commit 6ded275

File tree

5 files changed

+645
-674
lines changed

5 files changed

+645
-674
lines changed
 

‎bin/cli.js

+13-9
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@
2323
"migrate",
2424
"add",
2525
"remove",
26-
/*
27-
"update",
28-
"make",
29-
*/
3026
"serve",
3127
"generate-loader",
3228
"generate-plugin",
@@ -436,17 +432,17 @@ For more information, see https://webpack.js.org/api/cli/.`);
436432
if (argv.w) {
437433
compiler.hooks.watchRun.tap("WebpackInfo", compilation => {
438434
const compilationName = compilation.name ? compilation.name : "";
439-
console.log("\nCompilation " + compilationName + " starting…\n");
435+
console.error("\nCompilation " + compilationName + " starting…\n");
440436
});
441437
} else {
442438
compiler.hooks.beforeRun.tap("WebpackInfo", compilation => {
443439
const compilationName = compilation.name ? compilation.name : "";
444-
console.log("\nCompilation " + compilationName + " starting…\n");
440+
console.error("\nCompilation " + compilationName + " starting…\n");
445441
});
446442
}
447443
compiler.hooks.done.tap("WebpackInfo", compilation => {
448444
const compilationName = compilation.name ? compilation.name : "";
449-
console.log("\nCompilation " + compilationName + " finished\n");
445+
console.error("\nCompilation " + compilationName + " finished\n");
450446
});
451447
}
452448

@@ -491,8 +487,16 @@ For more information, see https://webpack.js.org/api/cli/.`);
491487
process.stdin.resume();
492488
}
493489
compiler.watch(watchOptions, compilerCallback);
494-
if (outputOptions.infoVerbosity !== "none") console.log("\nwebpack is watching the files…\n");
495-
} else compiler.run(compilerCallback);
490+
if (compiler.close) {
491+
compiler.close(compilerCallback);
492+
}
493+
if (outputOptions.infoVerbosity !== "none") console.error("\nwebpack is watching the files…\n");
494+
} else {
495+
compiler.run(compilerCallback);
496+
if (compiler.close) {
497+
compiler.close(compilerCallback);
498+
}
499+
}
496500
}
497501

498502
processOptions(options);

‎bin/convert-argv.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ module.exports = function(...args) {
7979
const ext = actualConfigFileName.replace(new RegExp(defaultConfigFileNames), "");
8080
configFiles.push({
8181
path: resolvedPath,
82-
ext,
82+
ext
8383
});
8484
}
8585
}

‎package-lock.json

+615-660
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+3-4
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
"bundler"
2323
],
2424
"files": [
25-
"bin"
25+
"bin",
26+
"scripts"
2627
],
2728
"scripts": {
2829
"bootstrap": "npm run clean:all && npm install && lerna bootstrap",
@@ -43,7 +44,7 @@
4344
"travis:lint": "npm run build && npm run lint && npm run tslint",
4445
"tslint": "tslint -c tslint.json \"packages/**/*.ts\"",
4546
"watch": "npm run build && tsc -w",
46-
"postinstall": "lightercollective"
47+
"postinstall": "./scripts/opencollective.sh"
4748
},
4849
"husky": {
4950
"hooks": {
@@ -111,10 +112,8 @@
111112
"enhanced-resolve": "^4.1.0",
112113
"findup-sync": "^2.0.0",
113114
"global-modules": "^1.0.0",
114-
"global-modules-path": "^2.3.0",
115115
"import-local": "^2.0.0",
116116
"interpret": "^1.1.0",
117-
"lightercollective": "^0.1.0",
118117
"loader-utils": "^1.1.0",
119118
"supports-color": "^5.5.0",
120119
"v8-compile-cache": "^2.0.2",

‎scripts/opencollective.sh

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/usr/bin/env bash
2+
3+
echo ''
4+
echo -e ' \x1B[1m***\x1B[0m Thank you for using webpack! \x1B[1m***\x1B[0m'
5+
echo ''
6+
echo 'Please consider donating to our open collective'
7+
echo ' to help us maintain this package.'
8+
echo ''
9+
echo ' https://opencollective.com/webpack/donate'
10+
echo ''
11+
echo -e ' \x1B[1m***\x1B[0m'
12+
echo ''
13+
exit 0

6 commit comments

Comments
 (6)

Izhaki commented on Mar 18, 2019

@Izhaki
Izhaki
on Mar 18, 2019

@evenstensberg

I wonder why the change from console.log to console.error in this commit?

This change breaks the tests for us and while we can modify the tests, we now have no way to distinguish a real error from a log entry.

Conceptually I can't work out the sense in using console.error for what is clearly not an error.

Concretely, we have this code:

    this.output = [];
    this.errors = [];
    this.childProcess.stdout.on('data', (data) => {
      console.log(data.toString());
      this.output.push(data.toString());
    });
    this.childProcess.stderr.on('data', (data) => {
      console.log(data.toString());
      this.errors.push(data.toString());
    });

Followed by:

After(function () {
  kill(this.childProcess.pid, 'SIGINT');
  if (this.errors.length > 0) {
    throw new Error(`Errors: ${this.errors}`);
  }
});

evenstensberg commented on Mar 19, 2019

@evenstensberg
evenstensberg
on Mar 19, 2019
MemberAuthor

Hi. We did it because when you used --json to pipe your arguments to, it would include the console log. @jhnns has some resources on why console.error is the best case here

Izhaki commented on Mar 20, 2019

@Izhaki
Izhaki
on Mar 20, 2019

OK. So in the case of:

webpack --json > stats.json

Why not disable the console.log()?

I mean, isn't --json a way to request JSON output rather than console.log()?

I also wonder how you got around this in tests? Are you not testing cases where there was an error? Say a bad webpack config?

evenstensberg commented on Mar 20, 2019

@evenstensberg
evenstensberg
on Mar 20, 2019
MemberAuthor

Yeah that would be an option, but in watch, watch will either way output that we’re watching the files, which I want to have there. We changed the tests

Izhaki commented on Mar 20, 2019

@Izhaki
Izhaki
on Mar 20, 2019

So is this a done deal?

Asking so to know whether to go ahead and update our tests.

evenstensberg commented on Mar 22, 2019

@evenstensberg
evenstensberg
on Mar 22, 2019
MemberAuthor

Yes

Please sign in to comment.