Skip to content

chore(logger): bumped consola #241

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.

Sign up for GitHub

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

Draft
YuryShkoda wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore(logger): bumped consola #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
46 changes: 25 additions & 21 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@types/cli-table": "0.3.0",
"@types/find": "0.2.1",
"@types/jest": "29.4.0",
"@types/node": "20.4.9",

@4gl-reviewer 4gl-reviewer Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Scope: The PR title says "chore(logger): bumped consola", but this commit also bumps typescript (4.5.4 to 5.1.6) and @types/node (17.0.12 to 20.4.9). These are significant upgrades that warrant mention in the PR title or description, since they introduce breaking type changes (see comment on line 52).

"@types/rimraf": "3.0.1",
"@types/valid-url": "1.0.3",
"babel-jest": "29.4.3",
Expand All @@ -48,15 +49,15 @@
"ts-jest": "29.0.5",
"ts-loader": "9.2.6",
"ts-node": "10.9.1",
"typescript": "4.5.4"
"typescript": "5.1.6"

@4gl-reviewer 4gl-reviewer Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correctness: Bumping typescript from 4.5.4 to 5.1.6 is a major version jump that introduces stricter type checking. This causes a compile error in src/file/spec/file.spec.ts:184:

error TS2345: Argument of type '"$"' is not assignable to parameter of type '"\\" | "/" | undefined'.

This happens because @types/node@20 types path.sep as '\\' | '/' (a literal union) rather than string as in @types/node@17. The unifyFilePath function in src/file/file.ts:166 defaults its separator parameter to path.sep, inheriting the narrow type. The test passes '$' which is no longer valid.

Fix: Widen the parameter type in unifyFilePath:

export function unifyFilePath(
  filePath: string,
  separator: string = path.sep,
  separatorToReplace = '/'
)

Note: ts-jest also warns that TypeScript 5.1.6 has not been tested with ts-jest 29.0.5 (supported range is >=4.3.0 <5.0.0).

},
"dependencies": {
"@fast-csv/format": "4.3.5",
"@types/fs-extra": "9.0.13",
"@types/prompts": "2.0.13",
"chalk": "4.1.1",
"cli-table": "0.3.6",
"consola": "2.15.0",
"consola": "3.2.3",
"find": "0.3.0",
"fs-extra": "10.0.0",
"jwt-decode": "3.1.2",
Expand Down
8 changes: 3 additions & 5 deletions src/logger/isLowerThanOrEqualTo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,20 @@ export const isLowerThanOrEqualTo = (
currentLevel: LogLevel,
level: LogLevel
): boolean => {
// REFACTOR: use switch

@4gl-reviewer 4gl-reviewer Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Style: The // REFACTOR: use switch comment looks like a development note/TODO. Either implement the switch refactor in this PR or remove the comment before merging, since leaving TODO-style comments without an associated issue reduces code clarity.

if (currentLevel === LogLevel.Off) return false

if (currentLevel === LogLevel.Debug) return true

if (currentLevel === LogLevel.Trace) return true

if (currentLevel === LogLevel.Info) {
return (
level === LogLevel.Info ||
level === LogLevel.Warn ||
level === LogLevel.Error
)
}

if (currentLevel === LogLevel.Warn)
if (currentLevel === LogLevel.Warn) {
return level === LogLevel.Warn || level === LogLevel.Error
}

return level === LogLevel.Error
}
2 changes: 1 addition & 1 deletion src/logger/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import consola from 'consola'
import { consola } from 'consola'

@4gl-reviewer 4gl-reviewer Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tests: The source file now uses the named import import { consola } from 'consola', but the test file src/logger/Logger.spec.ts still uses the default import import consola from 'consola'. While the tests currently pass (because in consola v3's CJS build consola.consola === consola), this inconsistency is fragile. If the module structure changes or the mock behavior diverges, the spy targets in the tests could stop matching the calls in the source. Recommend updating the test file to use the same named import style for consistency and robustness.

import { isLowerThanOrEqualTo } from './isLowerThanOrEqualTo'
import { isNullOrUndefined } from './isNullOrUndefined'
import { LogLevel } from './logLevel'
Expand Down