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
logger.ts undesirably monkey patches loglevel #667
Comments
@ghostec thanks for bringing this up. i can see why this becomes an issue across a project where the |
To get rid of the presence of It seems like implementing a plugin on the logger is the right path, but there's more to it. Instead of mutating the global interface LoggerMethod {
(...msg: any[]): void;
}
interface LoggerMethodFactory {
(methodName: LogLevel, logLevel: 0 | 1 | 2 | 3 | 4 | 5, loggerName: string): LoggerMethod;
}
// could do with some better naming, the idea is this function generates `LoggerMethodFactory`s
function loggerMethodFactoryFactory(name: string, originalFactory: LoggerMethodFactory): LoggerMethodFactory {
// return a LoggerMethodFactory
return (methodName: LogLevel, logLevel: 0 | 1 | 2 | 3 | 4 | 5, loggerName: string) => {
// chain with the original factory
const logMessage = originalFactory(methodName, logLevel, loggerName);
// return a LoggerMethod
return (...msg: any[]) => {
const segments = [`[${methodName.toUpperCase()}]`, loggerName].concat(msg);
// TODO: something with `name`?
// daisy chain with the original method factory
logMessage.apply(undefined, segments);
};
};
}
/**
* INTERNAL method for getting or creating a named Logger
*/
export function getLogger(name: string): Logger {
// get a logger id
const instanceId = instanceCount;
instanceCount += 1;
// get the logger instance and patch its methodFactory
const logger = log.getLogger(name + instanceId);
logger.methodFactory = logMethodFatoryFactory(name, logger.methodFactory);
return logger;
} This makes it so no globals are mutated by This also opens up "implement[ing] logger name prefixing", but I couldn't quite figure out what that meant since the logger's name is already included. |
@clavin this is great! if you're willing to work on that patch, we can land it. and yeah, i think the |
node-slack-sdk/src/logger.ts
Line 65 in 4024612
This will change the log format of projects that already use loglevel to "[INFO] undefined ..."
I had to revert the monkey patch in the file I'm importing @slack/client
The text was updated successfully, but these errors were encountered: