migrate to built-in logger #2563

hiroppy · 2020-05-02T12:55:01Z

DON'T MERGE but request maintainer's comments

For Bugs and Features; did you add new tests?

Yep

Motivation / Use-Case

We need to drop webpack-log because this package isn't under our management.
see https://www.npmjs.com/package/webpack-log

And webpack@4 supports infrastructureLogging system so I decided to migrate to it. See https://webpack.js.org/configuration/other-options/#infrastructurelogging.

Output

<i> [webpack-dev-server] Project is running at http://localhost:8080/
<i> [webpack-dev-server] webpack output is served from /
<i> [webpack-dev-server] Content not from webpack is served from /Users/hiroppy/webpack/webpack-dev-server

Dropped keys (CLI as well)

logLevel
quiet
info

Modified keys

clientLogLevel
- deleted debug, trace, silent, and warning instead of none, warn and verbose

Breaking Changes

Yep

Additional Info

Currently, I don't know why but the CLI test of multi ports fails so I'm investigating it.

* chore(deps): upgrade chokidar * chore(deps): switch to promise method without async for close * chore(ci): remove node v6 * chore(deps): fix issue of closing watchers before middleware * chore(deps): upgrade chokidar to v3.4.0

* chore(deps): upgrade deps * style: run prettier * test: update * ci: remove Node@8 * test(cli): add windows support * chore(deps): downgrade puppeteer * chore(deps): downgrade some deps * fix(hot): enable hot option as default (#2546) BREAKING CHANGE: the `hot` option is `true` by default, the `hotOnly` option was removed in favor `{ hot: 'only' }` * fix: remove lazy and filename options (#2544) BREAKING CHANGE: `lazy` and `filename` options was removed

codecov · 2020-05-02T13:00:44Z

Codecov Report

Merging #2563 into v4 will decrease coverage by 0.28%.
The diff coverage is 99.20%.

@@            Coverage Diff             @@
##               v4    #2563      +/-   ##
==========================================
- Coverage   93.77%   93.49%   -0.29%     
==========================================
  Files          34       34              
  Lines        1333     1275      -58     
  Branches      381      354      -27     
==========================================
- Hits         1250     1192      -58     
  Misses         81       81              
  Partials        2        2

Impacted Files	Coverage Δ
lib/utils/createConfig.js	`96.03% <ø> (-0.43%)`	⬇️
lib/utils/status.js	`90.00% <ø> (-1.31%)`	⬇️
client-src/default/index.js	`95.45% <92.85%> (+3.23%)`	⬆️
client-src/default/overlay.js	`97.01% <100.00%> (+0.04%)`	⬆️
client-src/default/utils/createLogger.js	`100.00% <100.00%> (ø)`
client-src/default/utils/reloadApp.js	`86.36% <100.00%> (-13.64%)`	⬇️
lib/Server.js	`96.73% <100.00%> (-0.06%)`	⬇️
lib/servers/SockJSServer.js	`93.75% <100.00%> (ø)`
lib/utils/addEntries.js	`100.00% <100.00%> (ø)`
lib/utils/createLogger.js	`100.00% <100.00%> (ø)`
... and 5 more

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 375ab23...b6af4e9. Read the comment docs.

hiroppy · 2020-05-02T12:55:39Z

bin/options.js

@@ -74,17 +74,12 @@ const options = {
    default: true,
    describe: 'Info',
  },
-  quiet: {


Respect to infrastructurelogging.level.

hiroppy · 2020-05-02T12:56:31Z

bin/options.js

  'client-log-level': {
    type: 'string',
    group: DISPLAY_GROUP,
    default: 'info',
    describe:
-      'Log level in the browser (trace, debug, info, warn, error or silent)',
+      'Log level in the browser (none, error, warn, info, log, verbose)',


We've already talked about it and they should respect infrastructurelogging.level.

hm, in fact, client-log-level should respect infrastructurelogging.level? What do you think?

hiroppy · 2020-05-02T12:58:43Z

client-src/default/index.js

    if (options.useWarningOverlay || options.useErrorOverlay) {
      overlay.clear();
    }
    sendMessage('StillOk');
  },
  'log-level': function logLevel(level) {
-    const hotCtx = require.context('webpack/hot', false, /^\.\/log$/);
-    if (hotCtx.keys().indexOf('./log') !== -1) {
-      hotCtx('./log').setLogLevel(level);


I'm not sure but I think this is unnecessary. I've already checked that this log-level is applied to clientLogLevel which is from CLI flag.

hiroppy · 2020-05-02T13:01:35Z

test/cli/cli.test.js

-        done: false,
-      },
-    };
+  it('should use different random port when multiple instances are started on different processes', async () => {


This test fails so I need to fix it.

Oh, this test in the master branch is broken as well...

cp1:null cp2: 8080

Yes this test passes because null !== 8080 but we don't expect it.

I've fixed this test.

hiroppy · 2020-05-02T15:20:57Z

lib/Server.js

@@ -38,13 +38,11 @@ if (!process.env.WEBPACK_DEV_SERVER) {
 }

 class Server {
-  constructor(compiler, options = {}, _log) {
+  constructor(compiler, options = {}, log) {


I remain log for users who want to inject a custom logger.

knagaitsev and others added 7 commits April 27, 2020 14:21

chore(deps): upgrade chokidar (#2539)

d85c473

* chore(deps): upgrade chokidar * chore(deps): switch to promise method without async for close * chore(ci): remove node v6 * chore(deps): fix issue of closing watchers before middleware * chore(deps): upgrade chokidar to v3.4.0

fix(run-open): introduce open instead of opn (#2532)

4e9b3db

test: support webapck5 (#2553)

f5498c9

chore(deps): update package-lock.json (#2551)

d29ddee

BREAKING CHANGE(server): get rid of conditions of Node version (#2552)

ecab24b

BREAKING CHANGE: remove inline and iframe/live mode (#2550)

308ee21

hiroppy commented May 2, 2020

View reviewed changes

hiroppy changed the title ~~BREAKING CHANGE: migrate on built-in logger~~ BREAKING CHANGE: migrate to built-in logger May 2, 2020

hiroppy force-pushed the feature/builtin-logger branch 3 times, most recently from 3252e64 to c2b23aa Compare May 2, 2020 15:29

hiroppy commented May 2, 2020

View reviewed changes

hiroppy force-pushed the feature/builtin-logger branch from c2b23aa to af1f64c Compare May 2, 2020 23:23

feat: migrate to built-in logger

b6af4e9

hiroppy force-pushed the feature/builtin-logger branch from af1f64c to b6af4e9 Compare May 3, 2020 00:15

alexander-akait force-pushed the v4 branch from 9c6a8e4 to 8775a07 Compare May 8, 2020 15:26

hiroppy changed the title ~~BREAKING CHANGE: migrate to built-in logger~~ migrate to built-in logger May 12, 2020

hiroppy closed this May 19, 2020

hiroppy deleted the feature/builtin-logger branch May 19, 2020 11:28

knagaitsev restored the feature/builtin-logger branch May 28, 2020 18:00

knagaitsev mentioned this pull request May 28, 2020

refactor(client): migrate client to built-in logger #2631

Merged

6 tasks

alexander-akait deleted the feature/builtin-logger branch February 27, 2021 14:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

migrate to built-in logger #2563

migrate to built-in logger #2563

Uh oh!

hiroppy commented May 2, 2020 •

edited

Loading

Uh oh!

codecov bot commented May 2, 2020 •

edited

Loading

Uh oh!

hiroppy May 2, 2020

Uh oh!

hiroppy May 2, 2020

Uh oh!

hiroppy May 2, 2020

Uh oh!

hiroppy May 2, 2020 •

edited

Loading

Uh oh!

hiroppy May 2, 2020

Uh oh!

hiroppy May 2, 2020

Uh oh!

hiroppy May 2, 2020

Uh oh!

hiroppy May 2, 2020

Uh oh!

Uh oh!

Uh oh!

migrate to built-in logger #2563

migrate to built-in logger #2563

Uh oh!

Conversation

hiroppy commented May 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

For Bugs and Features; did you add new tests?

Motivation / Use-Case

Output

Dropped keys (CLI as well)

Modified keys

Breaking Changes

Additional Info

Uh oh!

codecov bot commented May 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

hiroppy May 2, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hiroppy commented May 2, 2020 •

edited

Loading

codecov bot commented May 2, 2020 •

edited

Loading

hiroppy May 2, 2020 •

edited

Loading