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

Proposed fix for ./log module not found #1050

Merged
merged 3 commits into from Aug 23, 2017

Conversation

johan-lejdung
Copy link
Contributor

What kind of change does this PR introduce?
Bugfix of #1049

Did you add or update the examples/?
No

Summary
This is a proposed fix of the #1049
Instead of just assuming that the module ./log is loaded there is a check to see if it actually is.

Does this PR introduce a breaking change?
It should not!

Other information

@jsf-clabot
Copy link

jsf-clabot commented Aug 22, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Aug 22, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1050   +/-   ##
=======================================
  Coverage   72.25%   72.25%           
=======================================
  Files           4        4           
  Lines         465      465           
  Branches      139      139           
=======================================
  Hits          336      336           
  Misses        129      129

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 b2cf847...9a6e55b. Read the comment docs.

client/index.js Outdated
@@ -79,7 +79,7 @@ var onSocketMsg = {
},
"log-level": function(level) {
var hotCtx = require.context("webpack/hot", false, /^\.\/log$/);
if(hotCtx.keys().length > 0) {
if(hotCtx.keys().length > 0 && hotCtx.keys()["./log"]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see a small modification here please.

var contextKeys = hotCtx.keys();
if(contextKeys.length && contextKeys["./log"]) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely!

@shellscape
Copy link
Contributor

@johan-lejdung thanks for the PR! we have one small change request and then we'll be happy to merge.

@shellscape
Copy link
Contributor

Thanks!

@shellscape shellscape merged commit 0b4729f into webpack:master Aug 23, 2017
@johan-lejdung johan-lejdung deleted the fixNoModuleLogWarning branch August 23, 2017 12:30
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

3 participants