fix: cache child loggers

src/logger.ts

@@ -18,14 +18,36 @@ function makeLogger(namespace: string = OCLIF_NS): Logger {
   }
 }
 
-let cachedLogger: Logger | undefined
+/**
+ * Cache of logger instances. This is used to prevent creating multiple logger instances for the same namespace.
+ *
+ * The root logger is stored under the 'root' key as well as it's namespace.
+ */
+const cachedLoggers = new Map<string, Logger>()
 
+/**
+ * Returns a logger instance for the given namespace.
+ * If a namespace is provided, a child logger is returned.
+ * If no namespace is provided, the root logger is returned.
+ */
 export function getLogger(namespace?: string): Logger {
-  if (!cachedLogger) {
-    cachedLogger = makeLogger(OCLIF_NS)
+  let rootLogger = cachedLoggers.get('root')
+  if (!rootLogger) {
+    set(makeLogger(OCLIF_NS))
   }
 
-  return namespace && cachedLogger.namespace !== namespace ? cachedLogger.child(namespace) : cachedLogger
+  rootLogger = cachedLoggers.get('root')!
+
+  if (namespace) {
+    const cachedLogger = cachedLoggers.get(namespace)
+    if (cachedLogger) return cachedLogger
+
+    const logger = rootLogger.child(namespace)
+    cachedLoggers.set(namespace, logger)
+    return logger
+  }
+
+  return rootLogger
 }
 
 function ensureItMatchesInterface(newLogger: Logger): boolean {
@@ -41,10 +63,12 @@ function ensureItMatchesInterface(newLogger: Logger): boolean {
 }
 
 function set(newLogger: Logger): void {
-  if (cachedLogger) return
+  if (cachedLoggers.has(newLogger.namespace)) return
+  if (cachedLoggers.has('root')) return
 
   if (ensureItMatchesInterface(newLogger)) {
-    cachedLogger = newLogger
+    cachedLoggers.set(newLogger.namespace, newLogger)
+    cachedLoggers.set('root', newLogger)
   } else {
     process.emitWarning('Logger does not match the Logger interface. Using default logger.')
   }
@@ -64,3 +88,7 @@ export function setLogger(loadOptions: LoadOptions) {
     set(makeLogger(OCLIF_NS))
   }
 }
+
+export function clearLoggers() {
+  cachedLoggers.clear()
+}

test/logger.test.ts

@@ -0,0 +1,115 @@
+import {expect} from 'chai'
+import sinon from 'sinon'
+
+import {Config} from '../src/config/config'
+import {clearLoggers, getLogger, setLogger} from '../src/logger'
+
+describe('getLogger', () => {
+  let sandbox: sinon.SinonSandbox
+  let childStub: sinon.SinonStub
+  let config: Config
+
+  beforeEach(async () => {
+    clearLoggers()
+    sandbox = sinon.createSandbox()
+    childStub = sandbox
+      .stub()
+      .callsFake((ns: string, delimiter?: string) => customLogger(`MY_CLI${delimiter ?? ':'}${ns}`))
+    const customLogger = (namespace: string) => ({
+      child: childStub,
+      debug(_formatter: unknown, ..._args: unknown[]) {},
+      error(_formatter: unknown, ..._args: unknown[]) {},
+      info(_formatter: unknown, ..._args: unknown[]) {},
+      trace(_formatter: unknown, ..._args: unknown[]) {},
+      warn(_formatter: unknown, ..._args: unknown[]) {},
+      namespace,
+    })
+
+    config = await Config.load({
+      root: __dirname,
+      logger: customLogger('MY_CLI'),
+    })
+    setLogger(config)
+  })
+
+  afterEach(() => {
+    sandbox.restore()
+  })
+
+  it('should return the root logger if no namespace is provided', () => {
+    const logger = getLogger()
+    expect(logger.namespace).to.equal('MY_CLI')
+  })
+
+  it('should create a new logger if the namespace has not been created', () => {
+    const logger = getLogger('test')
+    expect(logger.namespace).to.equal('MY_CLI:test')
+    expect(childStub.withArgs('test').callCount).to.equal(1)
+  })
+
+  it('should return a cached logger if the namespace has already been created', () => {
+    getLogger('test')
+    expect(childStub.withArgs('test').callCount, 'first getLogger call should call .child()').to.equal(1)
+
+    const logger = getLogger('test')
+
+    expect(logger.namespace).to.equal('MY_CLI:test')
+    expect(childStub.withArgs('test').callCount, 'second getLogger call should not call .child()').to.equal(1)
+  })
+
+  it('should return default oclif logger if no custom logger is set', async () => {
+    clearLoggers()
+    const logger = getLogger()
+    expect(logger.namespace).to.equal('oclif')
+  })
+})
+
+describe('setLogger', () => {
+  const customLogger = (namespace: string) => ({
+    child: (ns: string, delimiter?: string) => customLogger(`${namespace}${delimiter ?? ':'}${ns}`),
+    debug(_formatter: unknown, ..._args: unknown[]) {},
+    error(_formatter: unknown, ..._args: unknown[]) {},
+    info(_formatter: unknown, ..._args: unknown[]) {},
+    trace(_formatter: unknown, ..._args: unknown[]) {},
+    warn(_formatter: unknown, ..._args: unknown[]) {},
+    namespace,
+  })
+
+  beforeEach(async () => {
+    clearLoggers()
+  })
+
+  it('should set the logger to the custom logger provided in loadOptions', async () => {
+    const config = await Config.load({
+      root: __dirname,
+      logger: customLogger('MY_CLI'),
+    })
+    setLogger(config)
+
+    const logger = getLogger()
+    expect(logger.namespace).to.equal('MY_CLI')
+  })
+
+  it('should set the logger to the default oclif logger if no custom logger is provided', async () => {
+    const config = await Config.load({
+      root: __dirname,
+    })
+    setLogger(config)
+
+    const logger = getLogger()
+    expect(logger.namespace).to.equal('oclif')
+  })
+
+  it('should use default logger if the provided logger does not match the Logger interface', async () => {
+    const logger = customLogger('MY_CLI')
+    // @ts-expect-error because we are testing an invalid logger interface
+    delete logger.child
+    const config = await Config.load({
+      root: __dirname,
+      logger,
+    })
+    setLogger(config)
+    const oclifLogger = getLogger()
+    expect(oclifLogger.namespace).to.equal('oclif')
+  })
+})