feat: typescript version by wayfarer3130 · Pull Request #209 · pimterry/loglevel

wayfarer3130 · 2026-01-21T14:00:02Z

This is a potential version for typescript - the api is compatible, but I haven't used much extra in terms of the typescript versioning of any ability to over-ride a class instance.

Note the lib/loglevel.js javascript version is GENERATED now from src/loglevel.ts. That is one of the things to discuss about what level of backwards compatibility this should have and how much new features should be taken advantage of.
Please provide comments about overall goals for a v2 - we can discuss the nitpicky details once we have the high level agreement on the general pattern.

wayfarer3130 · 2026-01-21T14:01:01Z

@pimterry - this is a potential version converted to typescript, mostly as a stalking horse to talk about to see if we can figure out the right changes for a v2 release.

Ryuno-Ki

I highlighted some areas where the spaces appear to be out of order.
Replacing vars can be a chore taken care of in another PR.

Ryuno-Ki · 2026-01-21T14:36:59Z

-  pull_request: {}
-  push:
-    branches:
-      - main


Removing this would make it run for pushes to any branch.

Ryuno-Ki · 2026-01-21T14:37:20Z

 *.gz
 *.tgz
+*.code-workspace
+package-lock.json


This file should be tracked in version control.

Ryuno-Ki · 2026-01-21T14:37:30Z

 *.tgz
+*.code-workspace
+package-lock.json
+bun.lock


This file should be tracked in version control.

Ryuno-Ki · 2026-01-21T14:42:45Z

-    var isIE = (typeof window !== undefinedType) && (typeof window.navigator !== undefinedType) && (
-        /Trident\/|MSIE /.test(window.navigator.userAgent)
-    );
+    const noop = function() {};


If you want to be consistent:

Suggested change

const noop = function() {};

const noop = function () {};

Ryuno-Ki · 2026-01-21T14:43:07Z

-    function realMethod(methodName) {
-        if (methodName === 'debug') {
-            methodName = 'log';
+    function defaultMethodFactory(methodName, _level, _loggerName) {


Suggested change

function defaultMethodFactory(methodName, _level, _loggerName) {

function defaultMethodFactory (methodName, _level, _loggerName) {

Ryuno-Ki · 2026-01-21T14:44:53Z

+         * in sync with the actual logging methods that we have installed (the
+         * parent could change levels but we might not have rebuilt the loggers
+         * in this child yet).
+         * @type {number}


Doesn't this conflict with the initialisation?

Suggested change

* @type {number}

* @type {number|null}

Ryuno-Ki · 2026-01-21T14:47:37Z

+
+        self.setDefaultLevel = function (level) {
+            defaultLevel = normalizeLevel(level);
+            if (getPersistedLevel()===undefined) {


Nitpick:

Suggested change

if (getPersistedLevel()===undefined) {

if (getPersistedLevel() === undefined) {

Ryuno-Ki · 2026-01-21T14:48:05Z

+        self.rebuild = function () {
+            inheritedLevel = null;
+            const result = replaceLoggingMethods.call(self);
+            if( self.methodFactory && self.methodFactory===(factory || defaultMethodFactory)) {


Suggested change

if( self.methodFactory && self.methodFactory===(factory || defaultMethodFactory)) {

if (self.methodFactory && self.methodFactory === (factory || defaultMethodFactory)) {

Ryuno-Ki · 2026-01-21T14:50:39Z

-                    window.console = mockConsole();
-                    log.trace("trace");
+        it("silent method calls are allowed", function() {
+            var result = log.setLevel(log.levels.SILENT);


Maybe worth to replace the vars here, too.

Ryuno-Ki · 2026-01-21T14:51:50Z

            });
+
+            it("child loggers inherit parent", function(log) {
+                log.setLevel(0);


Hm. This appears to be an implementation detail.

wayfarer3130 · 2026-01-21T15:03:59Z

I highlighted some areas where the spaces appear to be out of order. Replacing vars can be a chore taken care of in another PR.

The lib/loglevel.js file is now a generated file for compatibility - if v2 doesn't need that level of compatibility, then the ts can be the primary version and we can avoid generating a lib/loglevel.js Alterantively, we could leave lib/loglevel.ts as the primary typescript version, and add a compatible version in dist, and change the exports in package.json.

Ryuno-Ki · 2026-01-21T15:07:36Z

Ah, so that's what you meant with TS! I was confused as I haven't seen any changes to TS files.

pimterry · 2026-01-22T11:21:27Z

I'm very open to migrating to TS! Fully agree that would be good. It should happen on the V2 branch though, we're not migrating v1 to TS - it's frozen and there's risk for little benefit. This PR currently targets the main branch and seems to include all the other V2 changes here as well, which makes it hard to review. Can you update it to target v2, and only include the TS conversion?

Mr0grog · 2026-01-23T23:19:48Z

@wayfarer3130 sorry I’ve been so absent when I thought I’d have time to work on this. Glad someone’s taking it on.

Did you maybe push the wrong commit to your PR branch? This PR appears to be the exact same as the current v2 branch in this repo (same hash, so nothing’s been inserted into the history), and there’s no src/loglevel.ts file like you’re describing.

wayfarer3130 · 2026-01-24T17:48:50Z

@wayfarer3130 sorry I’ve been so absent when I thought I’d have time to work on this. Glad someone’s taking it on.

Did you maybe push the wrong commit to your PR branch? This PR appears to be the exact same as the current v2 branch in this repo (same hash, so nothing’s been inserted into the history), and there’s no src/loglevel.ts file like you’re describing.

I must have pushed it wrong somehow as it doesn't seem to have the changes I had locally. I will push that on Monday when I'm back on the computer I was working on.
Even when I do push it, I'm mostly thinking it is a discussion point, and is extremely unlikely to be what we use for v2, just that I wanted to get people talking about what they want in v2.

Ryuno-Ki · 2026-02-04T10:15:39Z

-    // Build the best logging method possible for this env
-    // Wherever possible we want to bind, not wrap, to preserve stack traces
-    function defaultMethodFactory(methodName, _level, _loggerName) {
+/*! loglevel - https://github.com/pimterry/loglevel - licensed MIT */


I'm not sure whether the comment can be changed into this form legally. Would it be possible to preserve the previous version?

Ryuno-Ki

Looks like it's on a good path. I feel like the typings could be more rigour.

Ryuno-Ki · 2026-02-04T10:17:28Z

+
+function defaultMethodFactory(methodName: LogMethodName): LoggingMethod {
+  if (typeof console !== undefinedType) {
+    const anyConsole = console as any;


Can we prefer unknown over any?

Ryuno-Ki · 2026-02-04T10:19:20Z

+  /*jshint validthis:true */
+  const level = this.getLevel();
+
+  for (let i = 0; i < logMethods.length; i++) {


My hunch is that a for…of loop is perhaps the right way?

Or perhaps even a logMethods.forEach((method, index) => {…})?

Ryuno-Ki · 2026-02-04T10:20:40Z

+
+  for (let i = 0; i < logMethods.length; i++) {
+    const methodName = logMethods[i];
+    this[methodName] = i < level ? noop : this.methodFactory(methodName, level, this.name);


The i should maybe be casted here. It feels kind of confusing that level is an integer in this line. (It makes me think which I interpret as a smell)

Ryuno-Ki · 2026-02-04T10:21:17Z

+  }
+}
+
+function Logger(this: any, name?: LoggerName, factory?: MethodFactory, parent?: any) {


Might benefit from a return type.

Ryuno-Ki · 2026-02-04T10:22:01Z

+function Logger(this: any, name?: LoggerName, factory?: MethodFactory, parent?: any) {
+  const self = this;
+
+  // Private instance variables:


JavaScript has private fields by now.

Ryuno-Ki · 2026-02-04T10:29:16Z

+
+  const loggers: any = {};
+
+  function persistLevelIfPossible(levelNum: number): void {


Might make sense to define a type for Indices like this:

// Source - https://stackoverflow.com/a/63904714 // Posted by bela53 // Retrieved 2026-02-04, License - CC BY-SA 4.0 type Indices<T extends readonly any[]> = Exclude<Partial<T>["length"], T["length"]>

Would then be

Suggested change

function persistLevelIfPossible(levelNum: number): void {

function persistLevelIfPossible(levelNum: Indices<typeof LogMethodName[]>): void {

if I read it correctly.

Ryuno-Ki · 2026-02-04T10:31:22Z

+    if (typeof window === undefinedType || !storageKey) return;
+
+    try {
+      (window as any).localStorage[storageKey] = levelName;


Is casting window really necessary here?
You check for typeof window above. Maybe there should be a general check for browser context before handling finer levels.

Ryuno-Ki · 2026-02-04T10:32:18Z

+
+  function getPersistedLevel(): string | void {
+    let storedLevel: any;
+    if (typeof window === undefinedType || !storageKey) return;


This starts to ask for a refactoring that handles the question of browser vs. server context generally.

Ryuno-Ki · 2026-02-04T10:35:13Z

-    "test-types": "tsc --noEmit ./test/type-test.ts && ts-node ./test/type-test.ts",
-    "dist": "grunt dist",
-    "dist-build": "grunt dist-build",
+    "test-types": "tsc --noEmit ./test/type-test.ts && ts-node --compilerOptions \"{\\\"module\\\":\\\"CommonJS\\\"}\" ./test/type-test.ts",


Do we really have to have this level of escaping?

Suggested change

"test-types": "tsc --noEmit ./test/type-test.ts && ts-node --compilerOptions \"{\\\"module\\\":\\\"CommonJS\\\"}\" ./test/type-test.ts",

"test-types": "tsc --noEmit ./test/type-test.ts && ts-node --compilerOptions '{\"module\":\"CommonJS\"}' ./test/type-test.ts",

feels more appropriate to me.

Ryuno-Ki · 2026-02-04T10:35:51Z

+import typescript from "@rollup/plugin-typescript";
+import terser from "@rollup/plugin-terser";
+
+const banner = `/*! loglevel - https://github.com/pimterry/loglevel - licensed MIT */`;


See my comment above on the shape of the banner.

Ryuno-Ki reviewed Jan 21, 2026

View reviewed changes

Missing files to actual review

01a7777

wayfarer3130 changed the base branch from main to v2 February 2, 2026 16:03

Ryuno-Ki reviewed Feb 4, 2026

View reviewed changes

	function defaultMethodFactory(methodName, _level, _loggerName) {
	function defaultMethodFactory (methodName, _level, _loggerName) {

	if (getPersistedLevel()===undefined) {
	if (getPersistedLevel() === undefined) {

	if( self.methodFactory && self.methodFactory===(factory \|\| defaultMethodFactory)) {
	if (self.methodFactory && self.methodFactory === (factory \|\| defaultMethodFactory)) {


		const loggers: any = {};

		function persistLevelIfPossible(levelNum: number): void {

	"test-types": "tsc --noEmit ./test/type-test.ts && ts-node --compilerOptions \"{\\\"module\\\":\\\"CommonJS\\\"}\" ./test/type-test.ts",
	"test-types": "tsc --noEmit ./test/type-test.ts && ts-node --compilerOptions '{\"module\":\"CommonJS\"}' ./test/type-test.ts",

Uh oh!

Conversation

wayfarer3130 commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wayfarer3130 commented Jan 21, 2026

Uh oh!

Ryuno-Ki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wayfarer3130 commented Jan 21, 2026

Uh oh!

Ryuno-Ki commented Jan 21, 2026

Uh oh!

pimterry commented Jan 22, 2026

Uh oh!

Mr0grog commented Jan 23, 2026

Uh oh!

wayfarer3130 commented Jan 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ryuno-Ki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wayfarer3130 commented Jan 21, 2026 •

edited

Loading