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

Add option to control browser log level #579

Merged
merged 3 commits into from
Sep 3, 2016
Merged

Conversation

SpaceK33z
Copy link
Member

Previously, when you used quiet: true or noInfo: true via the Node API or CLI, this only had effect on the output in the terminal (issue #109).

With this PR, these settings will be respected in the browser console as well.

Maybe it would be more flexible to add a separate option for this, but this would also mean more configuration. Looking for feedback on this.

Previously, when you used `quiet: true` or `noInfo: true` via the Node API or CLI, this only had effect on the output in the terminal (issue #109).

With this PR, these settings will be respected in the browser console as well.

Maybe it would be more flexible to add a separate option for this, but this would also mean more configuration. Looking for feedback on this.
@SpaceK33z
Copy link
Member Author

To test this:

Add this to your package.json:

"webpack-dev-server": "webpack/webpack-dev-server#browser-log-level",

After npm install, go to cd node_modules/webpack-dev-server, and run npm run prepublish.

for(var i = 0; i < errors.length; i++)
console.error(stripAnsi(errors[i]));
log("error", stripAnsi(errors[i]));
Copy link
Contributor

Choose a reason for hiding this comment

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

These are useful errors and I want them to be reported. They are related to user’s code.

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Hey, thanks for taking a look!

So my problem is there’s no way to separate useful output (compilation warnings and errors) from useless output (generally anything prefixed with WDS—our users have no idea what WDS even is, and they don’t need 20 messages about being disconnected either).

My guess is there’s some bug causing repeated disconnects. It just isn’t right that it keeps retrying and spamming the console IMO.

This option can be `error`, `warning`, `info` or `none`. It defaults to `info`.
It controls the log messages shown in the browser.
@SpaceK33z SpaceK33z changed the title Respect quiet and noInfo setting in browser console Add option to control browser log level Sep 2, 2016
@SpaceK33z
Copy link
Member Author

SpaceK33z commented Sep 2, 2016

@gaearon, I've updated the PR with your feedback.

  • I no longer abuse quiet and noInfo. Instead, there is a separate option, clientLogLevel. You can set this to error, warning, info or none. It defaults to info.
    • Via the CLI you can use this with --client-log-level. Not sure about the name btw, so suggestions are welcome.
  • Compilation errors will always be logged, disregarding the log level. Maybe there is a use-case where you don't want this, but we'll think about that when someone complains.
  • The repeated message [WDS] Disconnected! will now only display once.
    • Unfortunately there doesn't seem to be a way to hide the ERR_CONNECTION_REFUSED errors. This is happening because the websocket tries to re-connect every two seconds.

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Thanks for taking the time to work on this.

This is happening because the websocket tries to re-connect every two seconds.

Is this useful? Can we make it reconnect three times and give up? Or reconnect with exponential backoff?

@SpaceK33z
Copy link
Member Author

SpaceK33z commented Sep 2, 2016

Is this useful? Can we make it reconnect three times and give up? Or reconnect with exponential backoff?

If your PC gets into sleep mode, and a tab with the dev-server is still open, the connection would not be restored when you get out of sleep mode. We should be able to do something smarter with it, so I've made #584 for this. It's a bit outside the scope of this PR.

Do you think the rest of this PR is good?

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Yep, thanks.

@SpaceK33z SpaceK33z merged commit c544a26 into master Sep 3, 2016
@SpaceK33z SpaceK33z deleted the browser-log-level branch September 3, 2016 14:29
@gaearon
Copy link
Contributor

gaearon commented Sep 17, 2016

The only issue is we’re still on 1.x and can’t jump to 2.x until it’s stable.
Any chance you’d be interested in backporting this to 1.x?

@gaearon gaearon mentioned this pull request Sep 17, 2016
11 tasks
SpaceK33z added a commit that referenced this pull request Sep 17, 2016
@farwayer
Copy link

@SpaceK33z It will be very nice if this option can control HMR output also.

@ibrahima
Copy link

If I understand correctly, as @farwayer said the HMR output is still logged regardless, so we're still getting some unhelpful spam (which, in my case, is annoying because our front end tests get spammed with it constantly). Not sure whether it's even possible for that code to use the same logger since it's in a different project, but here's the source: https://github.com/webpack/webpack/blob/master/hot/dev-server.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants