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

Extracted client logging to its own module. #925

Closed
wants to merge 1 commit into from

Conversation

lbogdan
Copy link
Contributor

@lbogdan lbogdan commented May 29, 2017

This pull request opens the path to fixing the HMR logging issue described below.

What kind of change does this PR introduce?
This is basically refactoring.

Did you add or update the examples/?
Not needed.

Summary
There is a long standing issue with the HMR logging in the browser console, which cannot be turned off. This is because the modules in webpack/hot (dev-server.js, only-dev-server.js, log-apply-result.js) use console.log() and console.warn() directly, without any possibility to turn them off. This is especially annoying while trying to debug using console.log(), and getting your debug messages mixed with unrelated HMR messages. There are a few open issues about this, listed below, with some hackish workarounds.

To solve this once and for all, we should have an option to turn off HMR logging based on the desired log level. For this we can use the clientLogLevel option added in #579. But because the HMR modules don't have acces to webpack-dev-server/client/index.js's scope, where the clientLogLevel is defined, we have two options:

  • globally export clientLogLevel (or the logging function log()) from webpack-dev-server/client/index.js, and use it inside the webpack/hot modules, or
  • extract the logging code into its own module webpack-dev-server/client/log.js, and require it inside the webpack/hot modules, which I've used here.

As for the webpack/hot modules, instead of just having console.log("[HMR] some message here"), we will have something like

var log;

// is there a more elegant way of checking if a module exists?
var context = require.context("webpack-dev-server/client", false, /\.\/log$/);
if(context.keys().length > 0) {
  log = context("./log");
} else {
  log = function(level, msg) {
    var levels = {
      error: "error",
      info: "log",
      warning: "warn"
    };
    console[levels[level]](msg);
}
...
log("info", "[HMR] some message here");
...
log("warning", "[HMR] some warning here");
...

(also see the proof of concept PR here: webpack/webpack#4960)

Related issues list:
Option to console.log only important stuff #109
An option to mute console logging for hot module replacement
webpack/webpack#415
cannot mute webpack/hot/dev-server console.logs webpack/webpack#4115
How to quiet the console.log outputs? gaearon/react-hot-loader#79

Does this PR introduce a breaking change?
There's no breaking change, as far as I can see.

@jsf-clabot
Copy link

jsf-clabot commented May 29, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented May 29, 2017

Codecov Report

Merging #925 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #925   +/-   ##
=======================================
  Coverage   73.62%   73.62%           
=======================================
  Files           4        4           
  Lines         436      436           
  Branches      130      130           
=======================================
  Hits          321      321           
  Misses        115      115

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 662bc31...2b371ff. Read the comment docs.

@lbogdan
Copy link
Contributor Author

lbogdan commented Jun 1, 2017

Closed, opened #926.

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.

None yet

2 participants