Skip to content

fix: Logger has not export in entry file#1276

Merged
lamweili merged 1 commit intolog4js-node:masterfrom
taozi0818:taoz1/fix_types
Jul 6, 2022
Merged

fix: Logger has not export in entry file#1276
lamweili merged 1 commit intolog4js-node:masterfrom
taozi0818:taoz1/fix_types

Conversation

@taozi0818
Copy link
Copy Markdown
Contributor

@taozi0818 taozi0818 commented Jun 24, 2022

Fix #1275

Copy link
Copy Markdown
Contributor

@lamweili lamweili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

History in 2.0.0

It was changed from interface to class back in a1e40c2 which was released as 2.9.0.
It was probably due to the exposure of Logger as a global variable on configure().

Logger = loggerModule.Logger;

log4js-node/lib/logger.js

Lines 134 to 137 in a1e40c2

return {
LoggingEvent: LoggingEvent,
Logger: Logger
};

Refactored in 3.0.0

The global variable Logger has since been removed in 8084e80 which was released as 3.0.0.
But there was no change to the typings log4js.d.ts.


Thus, would a single line change from class to interface in log4js.d.ts (a reversal of a1e40c2) be better?

export class Logger {

-        export class Logger { 
+        export interface Logger { 

Because Logger should not be used standalone nor exposed. It won't work if log4js is not configured (which is also handled silently by getLogger()).

It is wrapped around and should only be retrieved from getLogger().

log4js-node/lib/log4js.js

Lines 144 to 154 in 3582a00

function getLogger(category) {
if (!enabled) {
configure(
process.env.LOG4JS_CONFIG || {
appenders: { out: { type: 'stdout' } },
categories: { default: { appenders: ['out'], level: 'OFF' } },
}
);
}
return new Logger(category || 'default');
}

@lamweili lamweili added this to the 6.5.3 milestone Jun 27, 2022
@taozi0818
Copy link
Copy Markdown
Contributor Author

OK. Both methods are available. The key point is keeping them consistent.

@lamweili
Copy link
Copy Markdown
Contributor

lamweili commented Jul 6, 2022

@taozi0818 Thanks! I rebased.

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.

Class Logger has not been export from entry file

2 participants