-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Set HMR log level. #926
Set HMR log level. #926
Conversation
Maybe we should check if var hotCtx = require.context("webpack/hot", false, /^\.\/log$/);
if(hotCtx.keys().length > 0) {
hotCtx("./log").setLogLevel(level);
} |
Codecov Report
@@ Coverage Diff @@
## master #926 +/- ##
=======================================
Coverage 72.13% 72.13%
=======================================
Files 4 4
Lines 463 463
Branches 139 139
=======================================
Hits 334 334
Misses 129 129 Continue to review full report at Codecov.
|
Looks good, but we need to wait for a pending major release before we merge. Didn't want to leave you hanging there. |
A webpack release, you mean? |
We're actually going to be doing a webpack-dev-server major release soon. It may correspond with webpack. |
Makes sense. It would be nice to be able to get rid of the |
@lbogdan since this module is going to continue to support older versions of webpack, we're going to need some error catching around this change. We can't just let it throw cause it'll affect those unable to upgrade webpack. once we get that in place I think we'll be good to go on this |
@shellscape The last commit has error catching (more like "feature" checking, actually). That's why the tests pass. Can you please check? |
@lbogdan cool, looks good to me. there's a conflict that needs resolving and then we can merge |
0e2c37c
to
89755e7
Compare
@shellscape Fixed conflicts, everything looks good. Also, I saw that with #921 |
Unfortunately I'm not privy to the workings of webpack main. It might be worth raising an issue there. |
Oh, ok, I was under the impression that |
I think there's some cross-polination between them all, but it's not 1:1 |
Second take at fixing the HMR logging issue (first was here) - not being able to turn off HMR's console logging. This change makes HMR respect
webpack-dev-server
'sclientLogLevel
.It uses
webpack/hot/log
merged here.What kind of change does this PR introduce?
This is a bugfix.
Did you add or update the
examples/
?None needed.
Summary
See #925.
Does this PR introduce a breaking change?
This needs a
webpack
version with webpack/webpack#4960 merged, otherwiserequire("webpack/hot/log")
will error. That's why the tests failed.