feat(webpack-cli): infrastructure logging

[piecyk]

What kind of change does this PR introduce?

Using InfrastructureLogger for logging output from webpack compiler

Did you add tests for your changes?

I plan to do it

If relevant, did you update the documentation?

No

Summary

Related #2019

Does this PR introduce a breaking change?

Yes

Other information

[piecyk]

As mention before webpack infrastructure logging outputs to stderr that affect's lot's of test.

[alexander-akait]

Sorry, it is big breaking change, do not touch stdout and stderr, it was big problems for previously versions and we fix it

[piecyk]

@evilebottnawi weback InfrastructureLogger is writing to stderr https://github.com/webpack/webpack/blob/master/lib/node/nodeConsole.js#L48

Aligned the utils/logger for simpler refactor test, and not go one by one 🙈 Let's revert logger to console and fix the test, just ping me if that is something we want as it's not a small thing.

[alexander-akait]

hm, I need dicussion with sokra

[alexander-akait]

Anyway we can't change it here, because it is breaking change

[piecyk]

Looks like reverting logger was not that bad. Still needed to disable few test, like with --bail is something off...

But yeah, also proposing some changes in what we log, now we show

Compilation finished at end, and simliar information we show in stats,
webpack 5.4.0 compiled successfully in 610 ms. Imho better to use this rather finished

something like

v4

$ webpack --watch --progress
<i> [webpack-cli] compilation starting...
<i> [webpack-cli] output:
<i> Hash: 01af0357780e7ff104a9
<i> Built at: 11/06/2020 12:41:15 PM
<i>   Asset      Size  Chunks             Chunk Names
<i> main.js  4.01 KiB    main  [emitted]  main
<i> Entrypoint main = main.js
<i> [./src/index.ts] 53 bytes {main} [built]
<i> [webpack-cli] webpack 4.44.2 compiled in 1871ms
<i> [webpack-cli] watching files for updates...

v5

$ webpack --watch --progress
<i> [webpack-cli] compilation starting...
<i> [webpack-cli] output:
<i> asset main.js 3.79 KiB [emitted] (name: main)
<i> runtime modules 668 bytes 3 modules
<i> ./src/index.ts 53 bytes [built] [code generated]
<i> [webpack-cli] webpack 5.4.0 compiled in 632 ms
<i> [webpack-cli] watching files for updates...

[alexander-akait]

Yes, we should remove Compilation finished

[alexander-akait]

<i> [webpack-cli] output: is unnecessary and looks like spam

[alexander-akait]

And yes, you will right we should - All diagnostic output goes to stderr. only the stats output goes to stdout

[alexander-akait]

No utils relates to complation, use methods classes

[piecyk]

This was execrated to utils as, moved stats display to WebpackCLIPlugin, but wanted to keep json output in WebpackCLI compiler callback.

[alexander-akait]

I think we should avoid do here a lot of changes, let's separate them, I can't remove and merge this all big changes, sorry, let's focus on migrate infrastructure logger

[codecov]

Codecov Report

Merging #2036 (b9a51fb) into master (8651f49) will increase coverage by 0.37%.
The diff coverage is 93.40%.

@@            Coverage Diff             @@
##           master    #2036      +/-   ##
==========================================
+ Coverage   68.31%   68.69%   +0.37%     
==========================================
  Files          77       79       +2     
  Lines        2408     2450      +42     
  Branches      496      507      +11     
==========================================
+ Hits         1645     1683      +38     
- Misses        763      767       +4     

Impacted Files	Coverage Δ
packages/webpack-cli/lib/webpack-cli.js	89.28% <82.35%> (-0.92%)	⬇️
packages/webpack-cli/lib/utils/stats-options.js	94.11% <94.11%> (ø)
...ckages/webpack-cli/lib/plugins/WebpackCLIPlugin.js	96.77% <96.36%> (-3.23%)	⬇️
packages/webpack-cli/lib/utils/name.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8651f49...b9a51fb. Read the comment docs.

[piecyk]

I think we should avoid do here a lot of changes, let's separate them, I can't remove and merge this all big changes, sorry, let's focus on migrate infrastructure logger

Yes i know. Basic problem is that migrating to infrastructure logger is bigger one as it touch a lot of testes as we switch from.

And yes, you will right we should - All diagnostic output goes to stderr. only the stats output goes to stdout

So basic when logging stats we should not use infrastructure logger ?

[alexander-akait]

Yes, for stats we should use stdout and do not use infrastructure logic

[alexander-akait]

Yes i know. Basic problem is that migrating to infrastructure logger is bigger one as it touch a lot of testes as we switch from.

So let's separate it

[piecyk]

So let's separate it

ok current plan

• revert utils/logger changes
• use infrastructure logger in context of webpack
• stats should log to stdout
• fix test

[snitin315]

We should set exitCode = 1 for errors.

[piecyk]

Exit code is set in callback in webpack-cli.js this wasn't changed https://github.com/webpack/webpack-cli/pull/2036/files#diff-7c711e2a4ef4b16d2cbbe1658495723402f9692da654cb6fa959127f3dc779f2R357-R359

Wondering if we should reset it if next complication is has no errors 🤔

[webpack-bot]

@piecyk Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@alexander-akait Please review the new changes.

[piecyk]

Suppressed by #2144