From 33648a711c2899722f77a04733c9c18748ca1d4e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:19:46 +0100 Subject: [PATCH] Remove deprecated `PrefixedLogger` interface (#4705) * Add some tests for `logger` * Remove deprecated `PrefixedLogger` interface `PrefixedLogger` has been deprecated for some time, so let's remove it now, while we have a major version bump. We can tidy up some of the other logic while we're here. Unfortunately lots of the code still uses `logger.log` which isn't exposed by the `Logger` interface, so we need to keep exposing that where it was before. --- spec/unit/logger.spec.ts | 51 ++++++++++++++++++++++++++++++++++++++ src/logger.ts | 53 ++++++++++++++++++++++------------------ 2 files changed, 80 insertions(+), 24 deletions(-) create mode 100644 spec/unit/logger.spec.ts diff --git a/spec/unit/logger.spec.ts b/spec/unit/logger.spec.ts new file mode 100644 index 000000000..263d7683a --- /dev/null +++ b/spec/unit/logger.spec.ts @@ -0,0 +1,51 @@ +/* +Copyright 2025 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* eslint-disable no-console */ + +import loglevel from "loglevel"; + +import { logger } from "../../src/logger.ts"; + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe("logger", () => { + it("should log to console by default", () => { + jest.spyOn(console, "debug").mockReturnValue(undefined); + logger.debug("test1"); + logger.log("test2"); + + expect(console.debug).toHaveBeenCalledWith("test1"); + expect(console.debug).toHaveBeenCalledWith("test2"); + }); + + it("should allow creation of child loggers which add a prefix", () => { + jest.spyOn(loglevel, "getLogger"); + jest.spyOn(console, "debug").mockReturnValue(undefined); + + const childLogger = logger.getChild("[prefix1]"); + expect(loglevel.getLogger).toHaveBeenCalledWith("matrix-[prefix1]"); + childLogger.debug("test1"); + expect(console.debug).toHaveBeenCalledWith("[prefix1]", "test1"); + + const grandchildLogger = childLogger.getChild("[prefix2]"); + expect(loglevel.getLogger).toHaveBeenCalledWith("matrix-[prefix1][prefix2]"); + grandchildLogger.debug("test2"); + expect(console.debug).toHaveBeenCalledWith("[prefix1][prefix2]", "test2"); + }); +}); diff --git a/src/logger.ts b/src/logger.ts index 5fc53611a..da381b47b 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -17,6 +17,18 @@ limitations under the License. import loglevel from "loglevel"; +/** Backwards-compatibility hack to expose `log` to applications that might still be relying on it. */ +interface LoggerWithLogMethod extends Logger { + /** + * Output debug message to the logger. + * + * @param msg - Data to log. + * + * @deprecated prefer {@link Logger.debug}. + */ + log(...msg: any[]): void; +} + /** Logger interface used within the js-sdk codebase */ export interface Logger extends BaseLogger { /** @@ -104,34 +116,29 @@ loglevel.methodFactory = function (methodName, logLevel, loggerName) { /** * Implementation of {@link Logger} based on `loglevel`. - * - * @deprecated this shouldn't be public; prefer {@link Logger}. */ -export interface PrefixedLogger extends loglevel.Logger, Logger { - /** @deprecated prefer {@link Logger.getChild} */ - withPrefix: (prefix: string) => PrefixedLogger; - - /** @deprecated internal property */ - prefix: string; +interface PrefixedLogger extends loglevel.Logger, LoggerWithLogMethod { + prefix?: string; } -/** Internal utility function to turn a `loglevel.Logger` into a `PrefixedLogger` */ -function extendLogger(logger: loglevel.Logger): void { - const prefixedLogger = logger; - prefixedLogger.getChild = prefixedLogger.withPrefix = function (prefix: string): PrefixedLogger { - const existingPrefix = this.prefix || ""; - return getPrefixedLogger(existingPrefix + prefix); - }; -} +/** + * Internal utility function: gets a {@link Logger} based on `loglevel`. + * + * Child loggers produced by {@link Logger.getChild} add the name of the child logger as a prefix on each log line. + * + * @param prefix Prefix to add to each logged line. If undefined, no prefix will be added. + */ +function getPrefixedLogger(prefix?: string): LoggerWithLogMethod { + const loggerName = DEFAULT_NAMESPACE + (prefix === undefined ? "" : `-${prefix}`); + const prefixLogger = loglevel.getLogger(loggerName) as PrefixedLogger; -function getPrefixedLogger(prefix: string): PrefixedLogger { - const prefixLogger = loglevel.getLogger(`${DEFAULT_NAMESPACE}-${prefix}`) as PrefixedLogger; - if (prefixLogger.prefix !== prefix) { - // Only do this setup work the first time through, as loggers are saved by name. - extendLogger(prefixLogger); + if (prefixLogger.getChild === undefined) { + // This is a new loglevel Logger which has not been turned into a PrefixedLogger yet. prefixLogger.prefix = prefix; + prefixLogger.getChild = (childPrefix): Logger => getPrefixedLogger((prefix ?? "") + childPrefix); prefixLogger.setLevel(loglevel.levels.DEBUG, false); } + return prefixLogger; } @@ -139,9 +146,7 @@ function getPrefixedLogger(prefix: string): PrefixedLogger { * Drop-in replacement for `console` using {@link https://www.npmjs.com/package/loglevel|loglevel}. * Can be tailored down to specific use cases if needed. */ -export const logger = loglevel.getLogger(DEFAULT_NAMESPACE) as PrefixedLogger; -logger.setLevel(loglevel.levels.DEBUG, false); -extendLogger(logger); +export const logger = getPrefixedLogger(); /** * A "span" for grouping related log lines together.