Browser console messages should respect clientLogLevel #921
Conversation
|
|
Codecov Report
@@ Coverage Diff @@
## master #921 +/- ##
=======================================
Coverage 72.13% 72.13%
=======================================
Files 4 4
Lines 463 463
Branches 139 139
=======================================
Hits 334 334
Misses 129 129Continue to review full report at Codecov.
|
There was a problem hiding this comment.
kudos for the PR and the effort. I'd like to see making using of https://www.npmjs.com/package/loglevel and removing the current log function, however. Is that something you'd be up for doing?
|
Absolutely, I can implement that shortly. |
|
I've rebased the change into a single commit to keep the history clean. Some defaults we were previously setting were able to be removed in lieu of loglevel's native default methods. Additionally a We we need to do a little funny business when receiving the log level from configuration, as Webpack's documentation specifies using the string An error will now be emitted if the configuration is attempting to set an unknown value for |
There was a problem hiding this comment.
Almost there, just one thing to clear up (see code comment). Please do sign the CLA (#921 (comment)) so we can get this one merged.
| var INFO = "info"; | ||
| var WARNING = "warning"; | ||
| var ERROR = "error"; | ||
| var NONE = "none"; |
There was a problem hiding this comment.
this is duplicating the same functionality as the quiet option though, isn't it? https://webpack.js.org/configuration/dev-server/#devserver-quiet-
There was a problem hiding this comment.
The specific lines you highlighted are not duplicating the quiet functionality, but absolutely our consumption of a log level of none (see https://webpack.js.org/configuration/dev-server/#devserver-clientloglevel for the documentation specifying the use of none) most certainly seems like duplicate functionality. I think this is a good question for the Webpack team, being that we technically have two options which will do roughly the same thing - should one of these options be deprecated?
A proposed solution is to deprecate the use of none as an option for clientLogLevel since that is technically not a log level (which the name clientLogLevel suggests), and leaving quiet flagged as true for completely silencing output.
The other option is to deprecate the use of quiet entirely and keep all logging options within the single configuration value of clientLogLevel.
* Implements loglevel npm package, removes native log() method
Removes console calls with internal log function which respects clientLogLevel configuration values.
Moves warnings and errors to the warning and error logs, respectively. Also utilizes constants for fewer string comparisons.
Resolves issue #855