-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix(browser): filter browser logging by level #2493
Conversation
type = type.toUpperCase() | ||
if (browserConsoleLogOptions.level) { | ||
var logPriority = constants.LOG_PRIORITIES.indexOf(browserConsoleLogOptions.level.toUpperCase()) | ||
if (constants.LOG_PRIORITIES[type] > logPriority) return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't constants.LOG_PRIORITIES[type]
a string, rather than a number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops...that was meant to be indexOf
- Do not log lower levels if `browserConsoleLogOptions.level` is set
df8a7d5
to
59abed4
Compare
@@ -14,6 +14,14 @@ exports.LOG_ERROR = 'ERROR' | |||
exports.LOG_WARN = 'WARN' | |||
exports.LOG_INFO = 'INFO' | |||
exports.LOG_DEBUG = 'DEBUG' | |||
exports.LOG_PRIORITIES = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you missed to add the 'LOG' level itself to constants.js,
exports.LOG_LOG = 'LOG'
exports.LOG_PRIORITIES = [
exports.LOG_DISABLE,
exports.LOG_ERROR,
exports.LOG_WARN,
exports.LOG_INFO,
exports.LOG_DEBUG,
exports.LOG_LOG
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no LOG_LOG
in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, but we need to add it.
LOG_LOG maps to 'console.log' function as well as LOG_DEBUG maps to 'console.debug'
Try your pull request with console.log and browserConsoleLogOptions.level = 'debug' and you will see that it doesn't work, cause
constants.LOG_PRIORITIES.indexOf(type)
will have -1 (not found)
Thank you |
browserConsoleLogOptions.level
is setThis should address #2228.