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

Improve category types #21

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions logtape/category.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export type CategoryList = Readonly<string[]>;

export type Category = string | CategoryList;
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add TSDoc comments for those two types?

Copy link
Author

Choose a reason for hiding this comment

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

done

3 changes: 2 additions & 1 deletion logtape/config.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { Category } from "./category.ts";
import { type FilterLike, toFilter } from "./filter.ts";
import type { LogLevel } from "./level.ts";
import { LoggerImpl } from "./logger.ts";
Expand Down Expand Up @@ -40,7 +41,7 @@ export interface LoggerConfig<
* The category of the logger. If a string, it is equivalent to an array
* with one element.
*/
category: string | string[];
category: Category;

/**
* The sink identifiers to use.
Expand Down
3 changes: 2 additions & 1 deletion logtape/formatter.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { CategoryList } from "./category.ts";
import type { LogLevel } from "./level.ts";
import util from "./nodeUtil.ts";
import type { LogRecord } from "./record.ts";
Expand Down Expand Up @@ -154,7 +155,7 @@ export interface TextFormatterOptions {
* If this is a function, it will be called with the category array and
* should return a string, which will be used for rendering the category.
*/
category?: string | ((category: readonly string[]) => string);
category?: string | ((category: CategoryList) => string);

/**
* The format of the embedded values.
Expand Down
30 changes: 12 additions & 18 deletions logtape/logger.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { Category, CategoryList } from "./category.ts";
import type { Filter } from "./filter.ts";
import type { LogLevel } from "./level.ts";
import type { LogRecord } from "./record.ts";
Expand All @@ -20,7 +21,7 @@ export interface Logger {
/**
* The category of the logger. It is an array of strings.
*/
readonly category: readonly string[];
readonly category: CategoryList;

/**
* The logger with the supercategory of the current logger. If the current
Expand All @@ -46,9 +47,7 @@ export interface Logger {
* @param subcategory The subcategory.
* @returns The child logger.
*/
getChild(
subcategory: string | readonly [string] | readonly [string, ...string[]],
): Logger;
getChild(subcategory: Category): Logger;
Copy link
Owner

Choose a reason for hiding this comment

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

We should not allow empty arrays here.

Copy link
Author

Choose a reason for hiding this comment

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

why?
I have a case where I generate category dynamically and it can be empty. In the case of this lib there is no real difference between returning current logger from getChild or returning a child as new instance. We just need to get the correct logger.

Copy link
Author

Choose a reason for hiding this comment

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

In my fork I even made that if a category is empty string, empty array, null or undefined then it will return current logger instead of new child
Seems ok by me)

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's misleading that the method name is getChild() but the object returned is clearly not a child logger, and I don't want the signature of the method to change. 🤔


/**
* Get a logger with contextual properties. This is useful for
Expand Down Expand Up @@ -385,7 +384,7 @@ export type LogTemplatePrefix = (
* with a single element.
* @returns The logger.
*/
export function getLogger(category: string | readonly string[] = []): Logger {
export function getLogger(category: Category = []): Logger {
return LoggerImpl.getLogger(category);
}

Expand All @@ -408,12 +407,12 @@ interface GlobalRootLoggerRegistry {
export class LoggerImpl implements Logger {
readonly parent: LoggerImpl | null;
readonly children: Record<string, LoggerImpl | WeakRef<LoggerImpl>>;
readonly category: readonly string[];
readonly category: CategoryList;
readonly sinks: Sink[];
parentSinks: "inherit" | "override" = "inherit";
readonly filters: Filter[];

static getLogger(category: string | readonly string[] = []): LoggerImpl {
static getLogger(category: Category = []): LoggerImpl {
let rootLogger: LoggerImpl | null = globalRootLoggerSymbol in globalThis
? ((globalThis as GlobalRootLoggerRegistry)[globalRootLoggerSymbol] ??
null)
Expand All @@ -425,10 +424,10 @@ export class LoggerImpl implements Logger {
}
if (typeof category === "string") return rootLogger.getChild(category);
if (category.length === 0) return rootLogger;
return rootLogger.getChild(category as readonly [string, ...string[]]);
return rootLogger.getChild(category);
}

private constructor(parent: LoggerImpl | null, category: readonly string[]) {
constructor(parent: LoggerImpl | null, category: CategoryList) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove the private keyword?

Copy link
Author

Choose a reason for hiding this comment

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

was playing with class inheritance. this can be rolled back

Copy link
Author

Choose a reason for hiding this comment

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

done

this.parent = parent;
this.children = {};
this.category = category;
Expand All @@ -437,10 +436,7 @@ export class LoggerImpl implements Logger {
}

getChild(
subcategory:
| string
| readonly [string]
| readonly [string, ...(readonly string[])],
subcategory: Category,
Copy link
Owner

Choose a reason for hiding this comment

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

We should not allow empty arrays here.

Copy link
Author

Choose a reason for hiding this comment

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

same as above

): LoggerImpl {
const name = typeof subcategory === "string" ? subcategory : subcategory[0];
const childRef = this.children[name];
Expand All @@ -456,9 +452,7 @@ export class LoggerImpl implements Logger {
if (typeof subcategory === "string" || subcategory.length === 1) {
return child;
}
return child.getChild(
subcategory.slice(1) as [string, ...(readonly string[])],
);
return child.getChild(subcategory.slice(1));
}

/**
Expand Down Expand Up @@ -683,7 +677,7 @@ export class LoggerCtx implements Logger {
this.properties = properties;
}

get category(): readonly string[] {
get category(): CategoryList {
return this.logger.category;
}

Expand All @@ -692,7 +686,7 @@ export class LoggerCtx implements Logger {
}

getChild(
subcategory: string | readonly [string] | readonly [string, ...string[]],
subcategory: Category,
Copy link
Owner

Choose a reason for hiding this comment

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

We should not allow empty arrays here.

Copy link
Author

Choose a reason for hiding this comment

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

same as above

): Logger {
return this.logger.getChild(subcategory).with(this.properties);
}
Expand Down
1 change: 1 addition & 0 deletions logtape/mod.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { type Category, type CategoryList } from "./category.ts";
export {
type Config,
ConfigError,
Expand Down
3 changes: 2 additions & 1 deletion logtape/record.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { CategoryList } from "./category.ts";
import type { LogLevel } from "./level.ts";

/**
Expand All @@ -7,7 +8,7 @@ export interface LogRecord {
/**
* The category of the logger that produced the log record.
*/
readonly category: readonly string[];
readonly category: CategoryList;

/**
* The log level.
Expand Down