Skip to content
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

Adding log level methods when using Logger class #2351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danilobatistaqueiroz
Copy link

@danilobatistaqueiroz danilobatistaqueiroz commented Sep 18, 2023

Moved the code for creation log level methods from create-logger.js to logger.js.
Encapsulated it in a new method named createLogLevelMethods.

I only changed the code for creation of log level methods in 3 places:

  1. From DerivedLogger.prototype[level] to Logger.prototype[level]

  2. Instead of calling Object.keys(opts.levels), now it calls Object.keys(levels) because I passed opts.levels as parameter named levels.

  3. Removed the function isLevelEnabledFunctionName, because it's used only in one place, I created a constant instead.

Added test for log level methods using new Logger()

Now inside create-logger.js it just calls: logger.createLogLevelMethods(logger, opts.levels);

Now it works when using the typescript version.

//index.ts
import { Logger, transports, format } from 'winston'
const logger = new Logger({
  level: 'info',
  transports: [new transports.Console()],
  format: format.combine(format.splat(), format.colorize({ all: true }), format.simple())
})

logger.warn('Hello world')
logger.info('Hello world')

export default logger

Related Issue:

Fixes: #2348


Suggestion:

Create a test suit using Typescript


Difference between the old code from the new one. (old on left, new on right) (old in create-logger.js, new in logger.js)

createLogLevelMethods

@DABH
Copy link
Contributor

DABH commented Jan 3, 2024

@danilobatistaqueiroz Do you think #2349 would also solve for adding the methods like you're trying to do here with your PR? Wondering if we can just merge that one or if we really need both of these

@danilobatistaqueiroz
Copy link
Author

@DABH , after compiling the typescript snippet example we have:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var winston_1 = require("winston");
var logger = new winston_1.Logger({
    level: 'info',
    transports: [new winston_1.transports.Console()],
    format: winston_1.format.combine(winston_1.format.splat(), winston_1.format.colorize({ all: true }), winston_1.format.simple())
});
logger.warn('Hello world');
logger.info('Hello world');
exports.default = logger;

Running using my commits, it will produce:

warn: Hello world
info: Hello world

Running using the commits in #2349 it will produce:

...../src/index.js:9
logger.warn('Hello world');
^

TypeError: logger.warn is not a function
at Object. (/..../src/index.js:9:8)
at Module._compile (node:internal/modules/cjs/loader:1233:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1287:10)
at Module.load (node:internal/modules/cjs/loader:1091:32)
at Module._load (node:internal/modules/cjs/loader:938:12)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
at node:internal/main/run_main_module:23:47

@danilobatistaqueiroz
Copy link
Author

danilobatistaqueiroz commented Jan 4, 2024

@DABH , Logger.prototype[level] = function (...args) { is called before each logger method, i.e.: logger.warn('Hello world'); if that function is defined on Logger class. (the reason I change from create-logger.js to logger.js)

But when defined in create-logger.js it isn't executed.
The problem is the definition of prototype function for Logger class.
The reference of Logger class when using create-logger.js isn't the same of Logger class used to create the mylogger variable.
When used inside create-logger.js the instance for logger isn't created yet.

When using the js version:

const mylogger = winston.createLogger({
  level: 'info',
  format: winston.format.json(),
  transports: [new winston.transports.Console()],
});

The prototype function is defined for the object mylogger and it works fine.
When using the typescript version:

const mylogger = new Logger({
  level: 'info',
  transports: [new transports.Console()],
  format: format.combine(format.splat(), format.colorize({ all: true }), format.simple())
})

It doesn't call createLogger for the Logger referred.

Obs: the anonymous function in create-logger.js is also called when importing winston: require('winston')
But the definitions of prototype functions aren't in the context for the mylogger variable.

A very simple solution could be calling the anonymous from create-logger.js inside the Logger constructor.
But doing it, the reference of Logger enter in conflict, the compiler isn't able to understand the Logger when extending DerivedLogger. Because there is another Logger in the context inside DerivedLogger.

i.e:

const createLogger = require('./create-logger');
...
class Logger ...  
  constructor(options) {
    super({ objectMode: true });
    this.configure(options);
    --> createLogger(options); //calling createLogger here doesn't work
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Typescript - info() is not a function
2 participants