feat: createLogger for easier logging in individual modules #5418
Conversation
Agree. I think it should be a candidate for deprecation (maybe not in this PR).
Love this idea. The only concern I have with logging history is the potential for memory leaks. For example, one might log a player object, which would immediately create a potential memory leak without them knowing. A history refactor could be involve solutions to that problem.
I like the idea of inheritance. The prefixes generate a kind of path allowing using to more easily filter the console. Player-Scoped LoggerAnother useful thing would be player-specific loggers: player.log('hello');
// > VIDEOJS: ${playerId}: helloThese could then be disposed with the player making some of the memory concerns with history a bit cleaner. |
|
We should just consider turning off history by default. Maybe even as part of this PR, I'm not sure that anyone is really using it and yeah, could be a memory issue. Having a player-scoped logger is a cool idea. |
|
Looks like the only |
|
Next is shared history, chainable loggers, and player log. |
|
Adding a |
|
Added |
gkatsev
changed the title
docs/guides/debugging.md
Outdated
| @@ -44,6 +49,23 @@ Similar to the `console`, any number of mixed-type values can be passed to `vide | |||
| videojs.log('this is a string', {butThis: 'is an object'}); | |||
| ``` | |||
|
|
|||
| ### Creating new Loggers | |||
|
|
|||
| Sometimes, you want to make a new module or plugin and log messages with a label. Kind of how all these logs are prepended with `VIDEOJS:`. You can do that via the `createLog` method. It takes a name and gives you back a log object like `videojs.log`. Here's an example: | |||
There was a problem hiding this comment.
createLog should be createLogger
|
|
||
| // Bail out if there's no console or if this type is not allowed by the | ||
| // current logging level. | ||
| if (!fn || !lvl || !lvlRegExp.test(type)) { |
There was a problem hiding this comment.
I don't recall which versions it is, but there are some IEs that don't allow using apply/call on console methods. Probably not the case in IE11, but might be worth checking.
There was a problem hiding this comment.
This code wasn't changed from what was around previously.
There was a problem hiding this comment.
Ah, then, it's probably safe. I could be showing my age again. 😄
gkatsev
force-pushed
the
create-logger
branch
from
d4f9256
to
bc70f3b
Compare
Create a new createLogger module for better debugging. Each logger has its own log level and its own createLogger that will nest logs underneath them. `player.log` is also included, which logs the player id as part of the log statement. The history API also got a filter method.
For example:
```js
var log = videojs.log.createLogger('foo');
log('hello');
// > VIDEOJS: foo: hello
```
This is the most naive implementation of transforming what we have into providing a create logger functionality.
Basically, right now,
log.jsis changed into acreate-logger.jswhich is a function that given a name returns a log function.log.jsnow usescreate-loggertocreateLogger('VIDEOJS')which is the default logger.videojs.createLoggeris exposed for users of the library.Currently, logByType is also exposed directly on the log object but that's probably not wanted.
Other tweaks we can consider: a shared/central history. This will allow us to make it be in one place and we could also provide some search functionality via the "main" logger. For example, searching for all things with the name of
VHSor something.Additionally, we may want to expose
createLoggeron thelogitself rather than onvideojsand have the logs inherit each time one is created: