fix: assign log levels to un bound functions#1483
Conversation
binding custom log level functions to `this` with arrow functions causes unintentional behavior. the expectation is that they should be callable the same way as logger.log if you extend a logger instance `logger.log` will be called on `logger` whereas `logger.warn` will be called on `logger.prototype` this fixes winstonjs#1482 but I believe it should be treated as a bug. I doubt blocking extension of only part of the surface area of the instance was intended. this sets the base for efficient child loggers that only extend write to add attributes.
|
This was probably an unintended side-effect of rewriting all our code in ES6. So good catch! :) If you could add probably just one test to show what wasn't working before (and prevent regressions), I'd be happy to merge this in. Thanks for your contribution! |
|
this test tests the shape of logged messages more than it has to. I only really need to check that the length of logged messages is 2. Please let me know if you'ed prefer i update it. I'm happy to. |
|
LGTM once CI passes! Thanks again. |
|
Would it be possible to get this fix in a release? |
|
Probably will get a release cut on Friday! |
|
Gentle reminder for a release 😀 |
|
@DABH @indexzero could we get this, or #1471 in a release? |
|
@ofrobots Yep, plan is to release both on 12/23 if no surprises come up. Sorry for delays -- schedule/life conflicts prevented maintainer meetings from happening in Oct/Nov. Thanks for bearing with us. |
* fix: assign log levels to un bound functions binding custom log level functions to `this` with arrow functions causes unintentional behavior. the expectation is that they should be callable the same way as logger.log if you extend a logger instance `logger.log` will be called on `logger` whereas `logger.warn` will be called on `logger.prototype` this fixes winstonjs#1482 but I believe it should be treated as a bug. I doubt blocking extension of only part of the surface area of the instance was intended. this sets the base for efficient child loggers that only extend write to add attributes. * test: test for custom method prototype binding * style
4 participants
binding custom log level functions to
thiswith arrow functions causes unintentional behavior.the expectation is that they should be callable the same way as logger.log
if you extend a logger instance
logger.logwill be called withloggerbeingthiswhereaslogger.warncan only be called withlogger.prototypeas this.this fixes #1482 but I believe it should be treated as a bug. I doubt blocking extension of only part of the surface area of the logger instance was intended.
this sets the base for efficient child loggers that only extend write to add attributes.
attn @DABH
//todo tests =)