feat: add logger package with customizable logging and plugin support#1
Conversation
WalkthroughThis update introduces a modular, extensible logger system with a new core logger package and a console logger plugin. It establishes type-safe plugin architecture, deep context merging, flexible message formatting, and colorized console output. The update also adds comprehensive tests, TypeScript type definitions, helper utilities, and configuration files for both logger and plugin packages. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Logger
participant Plugin
participant Console
App->>Logger: createLogger(options)
App->>Logger: use(consolePlugin)
App->>Logger: build()
App->>Logger: logger.info(message)
Logger->>Plugin: execute(ctx, priority, message, pipe)
Plugin->>Console: console.log(formattedMessage)
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
packages/logger/src/types/index.ts (1)
1-2: Same extension concern as inplugin/index.tsPlease apply/confirm the outcome of the previous comment here as well.
packages/logger/src/constant/index.ts (1)
1-2: Same extension concern as inplugin/index.tsPlease apply/confirm the outcome of the previous comment here as well.
🧹 Nitpick comments (8)
packages/logger/src/constant/log-level.ts (1)
1-33: Enum looks solid; consider future-proofing withFatal/Trace.Current levels cover most needs, but many ecosystems also expose either a
Fatal(higher priority thanError) or aTrace(lower thanVerbose) level. Adding placeholders now prevents disruptive re-ordering later since the numeric values are public API.packages/logger/tsconfig.json (1)
5-11: Re-evaluatejsx: "preserve"and emptypaths
- The logger package is non-React; switching from
react-jsxtopreservemay be unnecessary and can leak JSX in emitted JS if any sneaks in.- Declaring an empty
"paths": {}is redundant; omit the key unless you intend to add mappings.- Good call on adding
"node"totypes.packages/logger-plugin-console/src/types/type-options.ts (1)
6-6: Consider making the target function type more specific.The current type
(...args: any[]) => anyis very loose and doesn't provide type safety. Since this is meant to be a logging function, consider a more specific type:- target?: (...args: any[]) => any; + target?: (message: string) => void;This would provide better type safety and make the intended usage clearer.
packages/logger-plugin-console/src/helpers/helper-format-stack.ts (2)
3-10: Consider adding error handling for edge cases.The
parseStackfunction unconditionally removes the first line and processes remaining lines. Consider handling edge cases:function parseStack(stack: string) { + if (!stack || typeof stack !== 'string') { + return []; + } const lines = stack .split('\n') - .splice(1) + .slice(1) // Use slice instead of splice for immutability .map((l) => l.trim().replace('file://', '')); return lines; }
12-20: Improve regex handling for different stack trace formats.The regex assumes a specific format but won't handle all possible stack trace formats. Consider making it more robust:
export const formatStack = (stack: string) => `\n${parseStack(stack) .map( (line) => - ` ${line.replace(/^at ([\s\S]+) \((.+)\)/, (_, m1, m2) => - colors.gray(`at ${m1} (${colors.cyan(m2)})`) - )}` + ` ${line.replace(/^at ([\s\S]+?) \((.+)\)/, (_, m1, m2) => + colors.gray(`at ${m1} (${colors.cyan(m2)})`) + ).replace(/^at (.+)$/, (match, location) => + // Handle lines without parentheses (like "at <anonymous>") + colors.gray(match) + )}` ) .join('\n')}`;This handles both "at function (location)" and "at location" formats more gracefully.
packages/logger/src/helpers/helper-format-message.ts (1)
3-17: Simplify return type to avoid unnecessary wrapper object.The function returns
{ message: LoggerMessage }but could directly returnLoggerMessagefor simplicity. The wrapper object doesn't add value.Apply this diff to simplify the return type:
-export const formatMessage = <Message extends string | LoggerMessage>( - inputMsg: Message -): { - message: LoggerMessage; -} => { +export const formatMessage = <Message extends string | LoggerMessage>( + inputMsg: Message +): LoggerMessage => { let message: LoggerMessage; if (typeof inputMsg === 'string') { message = { message: inputMsg }; } else { message = inputMsg; } - return { - message, - }; + return message; };packages/logger-plugin-console/src/create-console-plugin.ts (1)
10-10: Consider adding explicit type annotation for ctx parameter.While TypeScript can infer the type, being explicit about
LoggerPluginContext<LoggerContext>would improve readability and match the pattern used elsewhere.Apply this diff for better type clarity:
- setup: (ctx: LoggerContext, level: LogLevel, message: LoggerMessage) => { + setup: (ctx: LoggerPluginContext<LoggerContext>, level: LogLevel, message: LoggerMessage) => {Don't forget to import the type:
-import { type LoggerContext } from '@hyperse/logger'; +import { type LoggerContext, type LoggerPluginContext } from '@hyperse/logger';packages/logger/src/plugin/create-plugin.ts (1)
5-14: Question the necessity of this wrapper function.The
createPluginfunction appears to be an unnecessary abstraction that simply wrapsdefinePlugin. ThePluginOptionsparameter is accepted but unused, and no additional logic is provided beyond callingdefinePlugin.Consider either:
- Removing this function and using
definePlugindirectly- Adding meaningful functionality that justifies the wrapper
- Documenting the intended future use of
PluginOptionsIf this wrapper is intended for future extensibility, consider adding documentation:
+/** + * Creates a logger plugin with optional plugin-specific options. + * @param plugin - The logger plugin to create + * @param _ - Plugin options (reserved for future use) + * @returns The configured plugin + */ export const createPlugin = <
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
package.json(1 hunks)packages/logger-plugin-console/package.json(2 hunks)packages/logger-plugin-console/src/create-console-plugin.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-create-context.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-create-icon.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-stack.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-formatter.ts(1 hunks)packages/logger-plugin-console/src/types/type-color.ts(1 hunks)packages/logger-plugin-console/src/types/type-options.ts(1 hunks)packages/logger-plugin-console/tests/console.ts(1 hunks)packages/logger-plugin-console/tests/index.test.ts(1 hunks)packages/logger/src/constant/constant.ts(1 hunks)packages/logger/src/constant/index.ts(1 hunks)packages/logger/src/constant/log-level.ts(1 hunks)packages/logger/src/core/create-logger.ts(1 hunks)packages/logger/src/core/index.ts(1 hunks)packages/logger/src/core/logger-builder.ts(1 hunks)packages/logger/src/helpers/helper-error-logger.ts(1 hunks)packages/logger/src/helpers/helper-execute-plugins.ts(1 hunks)packages/logger/src/helpers/helper-format-message.ts(1 hunks)packages/logger/src/helpers/helper-is-function.ts(1 hunks)packages/logger/src/helpers/index.ts(1 hunks)packages/logger/src/index.ts(1 hunks)packages/logger/src/plugin/create-plugin.ts(1 hunks)packages/logger/src/plugin/define-plugin.ts(1 hunks)packages/logger/src/plugin/index.ts(1 hunks)packages/logger/src/types/index.ts(1 hunks)packages/logger/src/types/type-logger-builder.ts(1 hunks)packages/logger/src/types/type-plugin.ts(1 hunks)packages/logger/tests/error.spec.ts(1 hunks)packages/logger/tests/index.test.ts(0 hunks)packages/logger/tests/sample.spec.ts(1 hunks)packages/logger/tests/test-utils.ts(1 hunks)packages/logger/tsconfig.json(1 hunks)packages/logger/vitest.config.ts(1 hunks)tsconfig.base.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/logger/tests/index.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (18)
packages/logger-plugin-console/src/helpers/helper-create-icon.ts (1)
packages/logger-plugin-console/src/types/type-color.ts (1)
Color(3-6)
packages/logger-plugin-console/src/helpers/helper-create-context.ts (1)
packages/logger-plugin-console/src/types/type-color.ts (1)
Color(3-6)
packages/logger/src/core/create-logger.ts (3)
packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)packages/logger/src/core/logger-builder.ts (1)
LoggerBuilder(24-115)
packages/logger-plugin-console/tests/index.test.ts (2)
packages/logger/src/core/create-logger.ts (1)
createLogger(5-12)packages/logger-plugin-console/src/create-console-plugin.ts (1)
createConsolePlugin(7-15)
packages/logger/src/helpers/helper-error-logger.ts (4)
packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger/src/constant/constant.ts (1)
defaultErrorLoggerName(5-5)
packages/logger/src/helpers/helper-execute-plugins.ts (2)
packages/logger/src/types/type-logger-builder.ts (2)
LoggerContext(4-7)LoggerPluginList(9-12)packages/logger/src/types/type-plugin.ts (2)
LoggerMessage(4-8)LoggerPluginContext(10-12)
packages/logger-plugin-console/tests/console.ts (2)
packages/logger/src/core/create-logger.ts (1)
createLogger(5-12)packages/logger-plugin-console/src/create-console-plugin.ts (1)
createConsolePlugin(7-15)
packages/logger/src/plugin/define-plugin.ts (2)
packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/types/type-plugin.ts (2)
LoggerMessage(4-8)LoggerPlugin(14-24)
packages/logger/src/helpers/helper-format-message.ts (1)
packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)
packages/logger-plugin-console/src/create-console-plugin.ts (5)
packages/logger-plugin-console/src/types/type-options.ts (1)
ConsoleOptions(1-11)packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)packages/logger-plugin-console/src/helpers/helper-formatter.ts (1)
createFormatter(19-59)
packages/logger/src/types/type-logger-builder.ts (1)
packages/logger/src/types/type-plugin.ts (2)
LoggerMessage(4-8)LoggerPlugin(14-24)
packages/logger/tests/test-utils.ts (1)
packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)
packages/logger-plugin-console/src/helpers/helper-formatter.ts (5)
packages/logger-plugin-console/src/types/type-color.ts (1)
Color(3-6)packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger-plugin-console/src/types/type-options.ts (1)
ConsoleOptions(1-11)packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)packages/logger-plugin-console/src/helpers/helper-format-stack.ts (1)
formatStack(12-20)
packages/logger/src/types/type-plugin.ts (1)
packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)
packages/logger/tests/error.spec.ts (4)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/types/type-plugin.ts (1)
LoggerMessage(4-8)packages/logger/src/core/create-logger.ts (1)
createLogger(5-12)
packages/logger/src/plugin/create-plugin.ts (3)
packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/types/type-plugin.ts (2)
LoggerMessage(4-8)LoggerPlugin(14-24)packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)
packages/logger/tests/sample.spec.ts (4)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger/tests/test-utils.ts (1)
setUpForTest(4-26)packages/logger/src/types/type-logger-builder.ts (1)
LoggerContext(4-7)packages/logger/src/core/create-logger.ts (1)
createLogger(5-12)
packages/logger/src/core/logger-builder.ts (7)
packages/logger/src/types/type-logger-builder.ts (2)
LoggerContext(4-7)LoggerPluginList(9-12)packages/logger/src/types/type-plugin.ts (2)
LoggerMessage(4-8)LoggerPlugin(14-24)packages/logger/src/constant/constant.ts (2)
defaultLogLevel(7-7)defaultLoggerName(3-3)packages/logger/src/helpers/helper-error-logger.ts (1)
createErrorLogger(6-16)packages/logger/src/helpers/helper-is-function.ts (1)
isFunction(7-9)packages/logger/src/helpers/helper-format-message.ts (1)
formatMessage(3-17)packages/logger/src/helpers/helper-execute-plugins.ts (1)
executePlugins(11-27)
🪛 GitHub Check: test (18.x)
packages/logger-plugin-console/tests/index.test.ts
[failure] 19-19: tests/index.test.ts > DEMO > should render
TypeError: [AsyncFunction bound executeLog] is not a spy or a call to a spy!
❯ tests/index.test.ts:19:25
🪛 GitHub Actions: CI-unitest
packages/logger-plugin-console/tests/index.test.ts
[error] 19-19: Test failure: TypeError: [AsyncFunction bound executeLog] is not a spy or a call to a spy! at line 19. The test 'DEMO > should render' failed due to improper spy usage.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy (20.x)
🔇 Additional comments (25)
package.json (1)
65-65: Confirm version & duplication with upstreamts-node.The scoped package
@hyperse/ts-nodeis added, but there is no regularts-nodepresent.
Please verify that all scripts (g:*, workspace packages, tests) rely on this scoped fork and that no transitive dependency still expects the canonicalts-nodepackage, otherwise resolution errors can surface in Yarn-PnP.packages/logger/vitest.config.ts (1)
13-14: Global 100 s timeout is generous – double-check necessity.If only a handful of integration tests are long-running, prefer
test.concurrentwith localtestTimeoutor--runInBandto avoid hiding real hangs in shorter suites.packages/logger/src/core/index.ts (1)
1-1: ESM-friendly re-export LGTM.Using the
.jsextension inside sources ensures the emitted code resolves correctly undernodeNext—good catch.tsconfig.base.json (1)
6-7:moduleResolution: "nodenext"– watch for tooling drift.Some older TS plugins (eslint-typescript, ts-node < 10.9) still default to
node16. Verify all linters, IDEs and the newly added@hyperse/ts-nodealign withnodenext; mixed modes cause duplicate module graphs.packages/logger/src/plugin/index.ts (1)
1-2: No action required: Node-style ESM resolution is enabledThe
packages/logger/tsconfig.jsonextendstsconfig.base.json, which sets"compilerOptions": { "moduleResolution": "nodenext" }With
moduleResolution: "nodenext", using.jsin re-export paths is correct and will resolve at runtime.packages/logger-plugin-console/package.json (1)
32-33: Localconsolescript depends on a workspace-level dev dependency
yarn node --import=@hyperse/ts-node/registerassumes@hyperse/ts-nodeis hoisted to the workspace root. When consumers install this package standalone (outside the monorepo), the script will fail.Consider adding
@hyperse/ts-nodeto this package’sdevDependenciesor guarding the script behind a workspace check.Also applies to: 48-51
packages/logger-plugin-console/src/helpers/helper-create-icon.ts (1)
4-8: LGTM! Clean and well-typed implementation.The function correctly uses dynamic property access on the
colorsobject with proper TypeScript typing. TheColortype constraint ensures type safety for theiconColorparameter.packages/logger/src/helpers/helper-is-function.ts (1)
1-9: LGTM! Standard utility function with excellent documentation.The implementation correctly uses
typeofto check for functions and includes comprehensive JSDoc documentation. This is a clean, reusable utility.packages/logger-plugin-console/src/helpers/helper-create-context.ts (1)
4-8: LGTM! Consistent implementation with proper styling.The function correctly applies background color and bold formatting to create badges. The implementation follows the same pattern as
createIconand uses proper TypeScript typing.packages/logger/src/helpers/index.ts (1)
1-4: LGTM! Standard barrel export file.The file correctly consolidates helper exports using the standard re-export pattern. The
.jsextensions are appropriate for ESM compatibility.packages/logger-plugin-console/src/types/type-color.ts (1)
1-6: LGTM! Clean type definition with proper exclusions.The type definition correctly extracts color method names from picocolors while excluding utility functions. The use of
Omitto exclude'createColors'and'isColorSupported'ensures only actual color methods are available in theColortype.packages/logger/src/core/create-logger.ts (1)
5-12: LGTM! Well-designed factory function with proper type constraints.The factory function correctly:
- Uses generic type parameters with sensible defaults
- Handles the context parameter to exclude
namebut allow optionalnameoverride- Returns a properly typed
LoggerBuilderinstance- Follows TypeScript best practices for generic factory methods
packages/logger-plugin-console/tests/console.ts (1)
1-23: LGTM! Comprehensive integration test covering key functionality.The test effectively covers:
- Logger creation with verbose level configuration
- Console plugin integration with timestamp option
- Multiple log levels (info, warn, debug, verbose)
- Error handling with proper error object structure including name, message, and stack
The test setup and usage patterns demonstrate the intended API usage well.
packages/logger/src/plugin/define-plugin.ts (2)
4-9: LGTM! Clean type-safe plugin definition helper.The generic type definition with default constraints is well-structured and provides excellent type safety for plugin creation.
12-14: LGTM! Simple and effective identity function.The implementation correctly serves as a type-safe wrapper that ensures plugin objects conform to the expected interface while maintaining full type information.
packages/logger-plugin-console/src/create-console-plugin.ts (1)
7-15: LGTM! Clean plugin implementation with proper separation of concerns.The console plugin is well-structured with proper use of the formatter helper and options handling. The setup method correctly implements the LoggerPlugin interface.
packages/logger/src/constant/constant.ts (1)
1-7: LGTM! Clean constants definition.The constants are well-defined with clear naming conventions and reasonable default values. Using
LogLevel.Infoas the default log level is a sensible choice for most applications.packages/logger/tests/test-utils.ts (1)
9-26: LGTM! Proper log level to console method mapping.The switch statement correctly maps each log level to the appropriate console method, with
LogLevel.Verboseappropriately usingconsole.log.packages/logger/src/types/type-logger-builder.ts (2)
4-7: LGTM! Well-designed LoggerContext type.The
LoggerContexttype captures the essential properties needed for logging context with appropriate types.
9-12: LGTM! Clean generic type definition.The
LoggerPluginListtype is well-designed with proper generic constraints, making it type-safe and flexible for different plugin implementations.packages/logger/tests/error.spec.ts (1)
8-62: LGTM! Comprehensive error handling test.This test thoroughly verifies the error handling behavior of the logger system:
- Properly creates a plugin that throws errors
- Correctly mocks the error logger to track invocations
- Tests all log levels to ensure consistent error propagation
- Validates both call count and parameter correctness
- Uses proper assertion patterns with
toMatchObjectandtoEqualThe test coverage ensures the logger's error handling mechanism works correctly across the plugin system.
packages/logger/src/index.ts (1)
1-5: Clean barrel export structureWell-organized module exports following best practices for package entry points.
packages/logger/src/types/type-plugin.ts (1)
1-24: Well-designed plugin type systemThe type definitions provide a solid foundation for the plugin architecture with proper generic constraints and async support.
packages/logger/tests/sample.spec.ts (1)
1-120: Comprehensive test coverage with proper mockingExcellent test structure with proper setup/teardown and comprehensive coverage of single and multiple plugin scenarios.
packages/logger/src/core/logger-builder.ts (1)
69-77: Priority comparison logic is correct given LogLevel definitionThe
LogLevelenum assigns lower numbers to more severe levels (Error = 0 … Verbose = 4). With a configured level of, say,Info (2), the checkif (priority > innerCtx.level) { return exitPipe(false); }correctly filters out less-severe (more verbose) messages (Debug = 3, Verbose = 4) while allowing Error, Warn, and Info through. Inverting this comparison would block the higher-severity logs instead. No change is needed here.
Likely an incorrect or invalid review comment.
| const logger = createLogger({ | ||
| name: 'test', | ||
| level: LogLevel.Verbose, | ||
| }) | ||
| .use(createConsolePlugin()) | ||
| .build(); | ||
|
|
||
| logger.info('Hello World'); | ||
| logger.error('Hello World'); | ||
| logger.warn('Hello World'); | ||
| logger.debug('Hello World'); | ||
| logger.verbose('Hello World'); | ||
|
|
||
| expect(logger.info).toHaveBeenCalledWith('Hello World'); | ||
| expect(logger.error).toHaveBeenCalledWith('Hello World'); | ||
| expect(logger.warn).toHaveBeenCalledWith('Hello World'); | ||
| expect(logger.debug).toHaveBeenCalledWith('Hello World'); | ||
| expect(logger.verbose).toHaveBeenCalledWith('Hello World'); | ||
| }); |
There was a problem hiding this comment.
Fix the test implementation - logger methods are not spies.
The test is failing because it's trying to use toHaveBeenCalledWith() on actual logger methods, not spies. The logger methods are real implementations, not mocked functions.
Solution 1: Mock the logger methods
it('should render', () => {
const logger = createLogger({
name: 'test',
level: LogLevel.Verbose,
})
.use(createConsolePlugin())
.build();
+ // Mock the logger methods
+ jest.spyOn(logger, 'info').mockImplementation(() => {});
+ jest.spyOn(logger, 'error').mockImplementation(() => {});
+ jest.spyOn(logger, 'warn').mockImplementation(() => {});
+ jest.spyOn(logger, 'debug').mockImplementation(() => {});
+ jest.spyOn(logger, 'verbose').mockImplementation(() => {});
logger.info('Hello World');
logger.error('Hello World');
logger.warn('Hello World');
logger.debug('Hello World');
logger.verbose('Hello World');
expect(logger.info).toHaveBeenCalledWith('Hello World');
expect(logger.error).toHaveBeenCalledWith('Hello World');
expect(logger.warn).toHaveBeenCalledWith('Hello World');
expect(logger.debug).toHaveBeenCalledWith('Hello World');
expect(logger.verbose).toHaveBeenCalledWith('Hello World');
});Solution 2: Test console output instead
it('should render', () => {
+ const consoleSpy = jest.spyOn(console, 'log').mockImplementation(() => {});
const logger = createLogger({
name: 'test',
level: LogLevel.Verbose,
})
.use(createConsolePlugin())
.build();
logger.info('Hello World');
logger.error('Hello World');
logger.warn('Hello World');
logger.debug('Hello World');
logger.verbose('Hello World');
- expect(logger.info).toHaveBeenCalledWith('Hello World');
- expect(logger.error).toHaveBeenCalledWith('Hello World');
- expect(logger.warn).toHaveBeenCalledWith('Hello World');
- expect(logger.debug).toHaveBeenCalledWith('Hello World');
- expect(logger.verbose).toHaveBeenCalledWith('Hello World');
+ expect(consoleSpy).toHaveBeenCalledTimes(5);
+ consoleSpy.mockRestore();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const logger = createLogger({ | |
| name: 'test', | |
| level: LogLevel.Verbose, | |
| }) | |
| .use(createConsolePlugin()) | |
| .build(); | |
| logger.info('Hello World'); | |
| logger.error('Hello World'); | |
| logger.warn('Hello World'); | |
| logger.debug('Hello World'); | |
| logger.verbose('Hello World'); | |
| expect(logger.info).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.error).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.warn).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.debug).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.verbose).toHaveBeenCalledWith('Hello World'); | |
| }); | |
| it('should render', () => { | |
| const logger = createLogger({ | |
| name: 'test', | |
| level: LogLevel.Verbose, | |
| }) | |
| .use(createConsolePlugin()) | |
| .build(); | |
| // Mock the logger methods | |
| jest.spyOn(logger, 'info').mockImplementation(() => {}); | |
| jest.spyOn(logger, 'error').mockImplementation(() => {}); | |
| jest.spyOn(logger, 'warn').mockImplementation(() => {}); | |
| jest.spyOn(logger, 'debug').mockImplementation(() => {}); | |
| jest.spyOn(logger, 'verbose').mockImplementation(() => {}); | |
| logger.info('Hello World'); | |
| logger.error('Hello World'); | |
| logger.warn('Hello World'); | |
| logger.debug('Hello World'); | |
| logger.verbose('Hello World'); | |
| expect(logger.info).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.error).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.warn).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.debug).toHaveBeenCalledWith('Hello World'); | |
| expect(logger.verbose).toHaveBeenCalledWith('Hello World'); | |
| }); |
🧰 Tools
🪛 GitHub Check: test (18.x)
[failure] 19-19: tests/index.test.ts > DEMO > should render
TypeError: [AsyncFunction bound executeLog] is not a spy or a call to a spy!
❯ tests/index.test.ts:19:25
🪛 GitHub Actions: CI-unitest
[error] 19-19: Test failure: TypeError: [AsyncFunction bound executeLog] is not a spy or a call to a spy! at line 19. The test 'DEMO > should render' failed due to improper spy usage.
🤖 Prompt for AI Agents
In packages/logger-plugin-console/tests/index.test.ts around lines 6 to 24, the
test incorrectly uses toHaveBeenCalledWith on real logger methods which are not
spies or mocks. To fix this, either mock or spy on the logger methods before
calling them so that toHaveBeenCalledWith can be used, or alternatively, test
the console output directly by capturing console logs during the test instead of
checking method calls.
| setup(ctx, __, message) { | ||
| console.error('error', ctx.pluginName, message.message); |
There was a problem hiding this comment.
Fix type mismatch in setup method parameters.
The setup method expects LoggerPluginContext<Context> but the parameter is typed as just Context. The code accesses ctx.pluginName which doesn't exist on LoggerContext.
Apply this diff to fix the type issue:
- setup(ctx, __, message) {
+ setup(ctx: LoggerPluginContext<Context>, __, message) {
console.error('error', ctx.pluginName, message.message);
},Or import the correct type:
+import type { LoggerPluginContext } from '../types/type-plugin.js';🤖 Prompt for AI Agents
In packages/logger/src/helpers/helper-error-logger.ts around lines 12 to 13, the
setup method's first parameter is incorrectly typed as Context, but it should be
LoggerPluginContext<Context> to access ctx.pluginName. Update the parameter type
to LoggerPluginContext<Context> by importing the correct type and applying it to
the setup method's first argument.
| for (const plugin of pluginList) { | ||
| const pluginCtx: LoggerPluginContext<Context> = { | ||
| ...ctx, | ||
| pluginName: plugin.name, | ||
| }; | ||
| await plugin.setup(pluginCtx, priority, message); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling to prevent plugin failures from breaking the logging pipeline.
Individual plugin failures will currently cause the entire logging operation to fail. Consider wrapping plugin execution in try-catch to ensure robustness.
Apply this diff to add error handling:
for (const plugin of pluginList) {
const pluginCtx: LoggerPluginContext<Context> = {
...ctx,
pluginName: plugin.name,
};
- await plugin.setup(pluginCtx, priority, message);
+ try {
+ await plugin.setup(pluginCtx, priority, message);
+ } catch (error) {
+ // Log plugin error but continue execution
+ console.error(`Plugin ${plugin.name} failed:`, error);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const plugin of pluginList) { | |
| const pluginCtx: LoggerPluginContext<Context> = { | |
| ...ctx, | |
| pluginName: plugin.name, | |
| }; | |
| await plugin.setup(pluginCtx, priority, message); | |
| } | |
| for (const plugin of pluginList) { | |
| const pluginCtx: LoggerPluginContext<Context> = { | |
| ...ctx, | |
| pluginName: plugin.name, | |
| }; | |
| try { | |
| await plugin.setup(pluginCtx, priority, message); | |
| } catch (error) { | |
| // Log plugin error but continue execution | |
| console.error(`Plugin ${plugin.name} failed:`, error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/logger/src/helpers/helper-execute-plugins.ts around lines 20 to 26,
the current code awaits each plugin's setup without error handling, causing any
plugin failure to break the entire logging pipeline. Wrap the await plugin.setup
call inside a try-catch block to catch and log errors for each plugin
individually, allowing the logging process to continue even if one plugin fails.
| let messageStr = ``; | ||
|
|
||
| if (showLoggerName) { | ||
| messageStr += `[${name}] `; | ||
| } | ||
|
|
||
| if (showTimestamp) { | ||
| messageStr += `${new Date().toISOString()} `; | ||
| } | ||
|
|
||
| function print(inputMessage: LoggerMessage) { | ||
| const { message, name, stack } = inputMessage; | ||
| if (name) { | ||
| messageStr += `${textColor(name)}: `; | ||
| } | ||
| if (message) { | ||
| messageStr += `${textColor(JSON.stringify(message))} `; | ||
| } | ||
| if (stack) { | ||
| messageStr += `${textColor(formatStack(stack))}`; | ||
| } | ||
| target(messageStr); | ||
| } |
There was a problem hiding this comment.
Fix message accumulation bug in print function
The messageStr variable is initialized outside the print function but modified inside it. This causes messages to accumulate across multiple print() calls, resulting in progressively longer output strings.
Move the message construction inside the print function:
- let messageStr = ``;
-
- if (showLoggerName) {
- messageStr += `[${name}] `;
- }
-
- if (showTimestamp) {
- messageStr += `${new Date().toISOString()} `;
- }
-
function print(inputMessage: LoggerMessage) {
+ let messageStr = ``;
+
+ if (showLoggerName) {
+ messageStr += `[${name}] `;
+ }
+
+ if (showTimestamp) {
+ messageStr += `${new Date().toISOString()} `;
+ }
+
const { message, name, stack } = inputMessage;
if (name) {
messageStr += `${textColor(name)}: `;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let messageStr = ``; | |
| if (showLoggerName) { | |
| messageStr += `[${name}] `; | |
| } | |
| if (showTimestamp) { | |
| messageStr += `${new Date().toISOString()} `; | |
| } | |
| function print(inputMessage: LoggerMessage) { | |
| const { message, name, stack } = inputMessage; | |
| if (name) { | |
| messageStr += `${textColor(name)}: `; | |
| } | |
| if (message) { | |
| messageStr += `${textColor(JSON.stringify(message))} `; | |
| } | |
| if (stack) { | |
| messageStr += `${textColor(formatStack(stack))}`; | |
| } | |
| target(messageStr); | |
| } | |
| function print(inputMessage: LoggerMessage) { | |
| let messageStr = ``; | |
| if (showLoggerName) { | |
| messageStr += `[${name}] `; | |
| } | |
| if (showTimestamp) { | |
| messageStr += `${new Date().toISOString()} `; | |
| } | |
| const { message, name, stack } = inputMessage; | |
| if (name) { | |
| messageStr += `${textColor(name)}: `; | |
| } | |
| if (message) { | |
| messageStr += `${textColor(JSON.stringify(message))} `; | |
| } | |
| if (stack) { | |
| messageStr += `${textColor(formatStack(stack))}`; | |
| } | |
| target(messageStr); | |
| } |
🤖 Prompt for AI Agents
In packages/logger-plugin-console/src/helpers/helper-formatter.ts between lines
32 and 54, the variable messageStr is declared outside the print function and
then appended inside it, causing message strings to accumulate across multiple
calls. To fix this, move the initialization of messageStr inside the print
function so that it resets to an empty string on each call before appending any
parts. This ensures each message is constructed fresh without carrying over
previous content.
| messageStr += `${textColor(name)}: `; | ||
| } | ||
| if (message) { | ||
| messageStr += `${textColor(JSON.stringify(message))} `; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON.stringify
JSON.stringify can throw when encountering circular references or other non-serializable values. Consider wrapping it in a try-catch block.
if (message) {
- messageStr += `${textColor(JSON.stringify(message))} `;
+ try {
+ messageStr += `${textColor(JSON.stringify(message))} `;
+ } catch (error) {
+ messageStr += `${textColor('[Circular or non-serializable object]')} `;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| messageStr += `${textColor(JSON.stringify(message))} `; | |
| if (message) { | |
| try { | |
| messageStr += `${textColor(JSON.stringify(message))} `; | |
| } catch (error) { | |
| messageStr += `${textColor('[Circular or non-serializable object]')} `; | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/logger-plugin-console/src/helpers/helper-formatter.ts at line 48,
the use of JSON.stringify on message can throw errors if the message contains
circular references or non-serializable values. Wrap the JSON.stringify call in
a try-catch block to safely handle such cases, and provide a fallback string or
error message in case of failure to prevent the application from crashing.
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (2)
packages/logger-common/LICENSE.md (1)
5-5: Grammar: replace “under below licenses” with “under the following license(s)”Current phrasing is ungrammatical (“under below”).
Proposed minimal fix:-This software is available under below licenses: +This software is available under the following license:packages/logger-common/src/helpers/helper-simple-deep-clone.ts (1)
5-7: Consider edge case: null-prototype objects.The
isObjectfunction correctly excludes arrays but may want to explicitly handle objects created withObject.create(null)which don't have a prototype. The current implementation handles this correctly, but a comment would be helpful.export function isObject(item: any): item is object { + // Handles regular objects and null-prototype objects created with Object.create(null) return item && typeof item === 'object' && !Array.isArray(item); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
logger-mono.code-workspace(2 hunks)packages/logger-common/.gitignore(1 hunks)packages/logger-common/.vscode/launch.json(1 hunks)packages/logger-common/.vscode/settings.json(1 hunks)packages/logger-common/CHANGELOG.md(1 hunks)packages/logger-common/LICENSE.md(1 hunks)packages/logger-common/README.md(1 hunks)packages/logger-common/eslint.config.js(1 hunks)packages/logger-common/index.js(1 hunks)packages/logger-common/lint-staged.config.mjs(1 hunks)packages/logger-common/package.json(1 hunks)packages/logger-common/src/constant/index.ts(1 hunks)packages/logger-common/src/constant/log-level.ts(1 hunks)packages/logger-common/src/helpers/helper-format-date.ts(1 hunks)packages/logger-common/src/helpers/helper-merge-options.ts(1 hunks)packages/logger-common/src/helpers/helper-simple-deep-clone.ts(1 hunks)packages/logger-common/src/helpers/helper-sleep.ts(1 hunks)packages/logger-common/src/helpers/index.ts(1 hunks)packages/logger-common/src/index.ts(1 hunks)packages/logger-common/src/types/index.ts(1 hunks)packages/logger-common/src/types/type-message.ts(1 hunks)packages/logger-common/src/types/type-partial-deep.ts(1 hunks)packages/logger-common/tsconfig.build.json(1 hunks)packages/logger-common/tsconfig.json(1 hunks)packages/logger-common/tsup.config.ts(1 hunks)packages/logger-common/vitest.config.ts(1 hunks)packages/logger-plugin-console/package.json(2 hunks)packages/logger-plugin-console/src/constant.ts(1 hunks)packages/logger-plugin-console/src/create-console-plugin.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-assert-message.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-message.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-get-color-applier.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-merge-optons.ts(1 hunks)packages/logger-plugin-console/src/types/type-options.ts(1 hunks)packages/logger-plugin-console/src/types/type-plugin.ts(1 hunks)packages/logger-plugin-console/tests/console.ts(1 hunks)packages/logger-plugin-console/tsconfig.json(1 hunks)packages/logger-plugin-console/vitest.config.ts(1 hunks)packages/logger/package.json(1 hunks)packages/logger/src/constant/constant.ts(1 hunks)packages/logger/src/constant/index.ts(1 hunks)packages/logger/src/core/create-logger.ts(1 hunks)packages/logger/src/core/logger.ts(1 hunks)packages/logger/src/helpers/helper-execute-fun.ts(1 hunks)packages/logger/src/helpers/index.ts(1 hunks)packages/logger/src/plugin/define-plugin.ts(1 hunks)packages/logger/src/types/index.ts(1 hunks)packages/logger/src/types/type-logger-plugin.ts(1 hunks)packages/logger/src/types/type-logger.ts(1 hunks)packages/logger/tests/ctx.spec.ts(1 hunks)packages/logger/tests/error.spec.ts(1 hunks)packages/logger/tests/logger.spec.ts(1 hunks)packages/logger/tests/test-utils.ts(1 hunks)packages/logger/tsconfig.json(1 hunks)packages/logger/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (31)
- packages/logger-common/.vscode/settings.json
- packages/logger-common/README.md
- logger-mono.code-workspace
- packages/logger-common/CHANGELOG.md
- packages/logger-common/.gitignore
- packages/logger-common/src/constant/index.ts
- packages/logger-common/src/index.ts
- packages/logger-common/eslint.config.js
- packages/logger/src/helpers/index.ts
- packages/logger-common/src/helpers/index.ts
- packages/logger-common/index.js
- packages/logger-common/src/helpers/helper-sleep.ts
- packages/logger-plugin-console/tsconfig.json
- packages/logger-common/.vscode/launch.json
- packages/logger-common/src/types/index.ts
- packages/logger-common/src/constant/log-level.ts
- packages/logger-common/src/helpers/helper-format-date.ts
- packages/logger-common/vitest.config.ts
- packages/logger-plugin-console/tests/console.ts
- packages/logger-common/tsup.config.ts
- packages/logger-common/tsconfig.build.json
- packages/logger-common/tsconfig.json
- packages/logger-plugin-console/src/types/type-plugin.ts
- packages/logger-common/src/types/type-partial-deep.ts
- packages/logger-plugin-console/src/helpers/helper-merge-optons.ts
- packages/logger-plugin-console/src/constant.ts
- packages/logger-plugin-console/src/helpers/helper-get-color-applier.ts
- packages/logger-common/src/types/type-message.ts
- packages/logger-common/package.json
- packages/logger/src/types/type-logger-plugin.ts
- packages/logger/src/types/type-logger.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/logger/src/constant/index.ts
- packages/logger/vitest.config.ts
- packages/logger/tsconfig.json
- packages/logger-plugin-console/package.json
- packages/logger/src/plugin/define-plugin.ts
- packages/logger/tests/test-utils.ts
- packages/logger-plugin-console/src/create-console-plugin.ts
🧰 Additional context used
🧬 Code Graph Analysis (9)
packages/logger-plugin-console/src/helpers/helper-assert-message.ts (1)
packages/logger-plugin-console/src/types/type-plugin.ts (1)
ConsolePluginMessage(6-6)
packages/logger-plugin-console/src/types/type-options.ts (1)
packages/logger-plugin-console/src/types/type-color.ts (1)
Color(3-6)
packages/logger/tests/error.spec.ts (4)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(13-15)packages/logger/src/types/type-logger.ts (1)
LoggerContext(4-7)packages/logger/src/core/create-logger.ts (1)
createLogger(5-15)packages/logger-common/src/helpers/helper-sleep.ts (1)
sleep(1-5)
packages/logger/tests/ctx.spec.ts (4)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(13-15)packages/logger/src/types/type-logger.ts (1)
LoggerContext(4-7)packages/logger/src/core/create-logger.ts (1)
createLogger(5-15)packages/logger-common/src/helpers/helper-sleep.ts (1)
sleep(1-5)
packages/logger/src/core/create-logger.ts (4)
packages/logger/src/types/type-logger.ts (1)
LoggerContext(4-7)packages/logger-common/src/types/type-message.ts (1)
LoggerMessage(8-8)packages/logger-common/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger/src/core/logger.ts (2)
error(114-120)Logger(23-129)
packages/logger-common/src/helpers/helper-merge-options.ts (2)
packages/logger-common/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger-common/src/helpers/helper-simple-deep-clone.ts (3)
simpleDeepClone(21-51)isObject(5-7)isClassInstance(9-15)
packages/logger-plugin-console/src/helpers/helper-format-message.ts (5)
packages/logger-plugin-console/src/types/type-plugin.ts (2)
ConsolePluginContext(4-4)ConsolePluginMessage(6-6)packages/logger-plugin-console/src/types/type-options.ts (1)
ConsoleOptions(4-121)packages/logger-plugin-console/src/constant.ts (1)
defaultLevelColor(5-11)packages/logger-plugin-console/src/helpers/helper-get-color-applier.ts (1)
getColorApplier(24-37)packages/logger-plugin-console/src/helpers/helper-format-stack.ts (1)
formatStack(12-20)
packages/logger/tests/logger.spec.ts (5)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(13-15)packages/logger/tests/test-utils.ts (1)
setUpForTest(4-26)packages/logger/src/types/type-logger.ts (1)
LoggerContext(4-7)packages/logger/src/core/create-logger.ts (1)
createLogger(5-15)packages/logger-common/src/helpers/helper-sleep.ts (1)
sleep(1-5)
packages/logger/src/core/logger.ts (7)
packages/logger/src/types/type-logger.ts (2)
LoggerContext(4-7)BaseLogger(14-28)packages/logger-common/src/types/type-message.ts (1)
LoggerMessage(8-8)packages/logger-common/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger/src/constant/constant.ts (2)
defaultLoggerName(3-3)defaultLogLevel(7-7)packages/logger-common/src/helpers/helper-merge-options.ts (1)
mergeOptions(34-75)packages/logger/src/helpers/helper-execute-fun.ts (1)
executeFunction(7-16)packages/logger/src/types/type-logger-plugin.ts (1)
LoggerPlugin(9-20)
🪛 Biome (1.9.4)
packages/logger-common/src/helpers/helper-simple-deep-clone.ts
[error] 46-46: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 31-31: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 GitHub Check: test (18.x)
packages/logger-plugin-console/src/helpers/helper-format-message.ts
[warning] 31-31:
'use24HourClock' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 30-30:
'showTimestampRelativeToLastLog' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 28-28:
'showRelativeTimestamp' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 27-27:
'showMonthBeforeDay' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 26-26:
'showDate' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 25-25:
'capitalizeLevelName' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 22-22:
'showPluginName' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 18-18:
'pluginName' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 18-18:
'loggerName' is assigned a value but never used. Allowed unused vars must match /^_/u
[warning] 17-17:
'time' is assigned a value but never used. Allowed unused vars must match /^_/u
packages/logger/src/constant/constant.ts
[failure] 1-1: tests/index.test.ts
Error: Cannot find module '@hyperse/logger-common' imported from '/home/runner/work/logger/logger/packages/logger/src/constant/constant.ts'.
- If you rely on tsconfig.json's "paths" to resolve modules, please install "vite-tsconfig-paths" plugin to handle module resolution.
- Make sure you don't have relative aliases in your Vitest config. Use absolute paths instead. Read more: https://vitest.dev/guide/common-errors
❯ ../logger/src/constant/constant.ts:1:1
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Serialized Error: { code: 'ERR_MODULE_NOT_FOUND' }
🪛 LanguageTool
packages/logger-common/LICENSE.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ... Ltd) This software is available under below licenses: - Hyperse Commercial License...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
packages/logger-common/LICENSE.md
14-14: Bare URL used
(MD034, no-bare-urls)
🪛 GitHub Actions: CI-unitest
packages/logger/src/constant/constant.ts
[error] 1-1: Cannot find module '@hyperse/logger-common' imported from this file. If relying on tsconfig.json 'paths', install 'vite-tsconfig-paths' plugin for module resolution. Avoid relative aliases in Vitest config; use absolute paths instead.
🔇 Additional comments (14)
packages/logger-common/LICENSE.md (2)
7-9: Plural-singular mismatch between header and bullet listLine 5 uses the plural “licenses” but only a single licence is listed (HCL).
Either (a) keep the plural and add any other applicable licences (e.g., OSS licence), or (b) change the wording to singular as shown in the previous comment.
Please align to avoid reader confusion.
11-14: Different legal entities referenced—please verify accuracyThe notice mentions:
• “Hyperse (Yuantai Information Technology (Shanghai) Co., Ltd)”
• later “Hyperse GmbH”If both entities should be named, clarify their respective roles (e.g., copyright holder vs. licensor).
If not, adjust to a single, correct legal entity to prevent contractual ambiguity.packages/logger-plugin-console/src/helpers/helper-assert-message.ts (1)
3-14: LGTM! Clean and well-implemented message normalizer.The function correctly handles both string and object inputs, ensuring consistent message structure for the console plugin pipeline. The type safety and logic are sound.
packages/logger-plugin-console/src/types/type-options.ts (1)
4-121: Excellent comprehensive type definition with thorough documentation.The
ConsoleOptionstype is well-structured with clear JSDoc documentation, practical examples, and default values. The type safety for color mappings and the comprehensive coverage of console logging options make this a solid foundation for the console plugin.packages/logger/src/types/index.ts (1)
1-2: LGTM! Clean barrel export pattern.The export structure follows standard barrel export patterns for organizing type definitions. The
.jsextensions in the import paths are correct for ESM compatibility in TypeScript.packages/logger/package.json (1)
47-47: LGTM! Correct workspace dependency setup.The workspace dependency syntax is appropriate for monorepo packages and will ensure proper resolution during development and publishing.
packages/logger-plugin-console/vitest.config.ts (1)
9-12: LGTM! Consistent alias pattern for test resolution.The alias addition follows the same pattern as the existing
@hyperse/loggeralias and is necessary for proper module resolution in the test environment.packages/logger/src/helpers/helper-execute-fun.ts (1)
7-16: LGTM! Well-implemented generic function executor.The function correctly handles optional functions with proper TypeScript generics and type safety. The implementation appropriately awaits the function execution to handle both synchronous and asynchronous functions.
The early return for undefined functions is clean and efficient.
packages/logger/src/constant/constant.ts (2)
3-7: LGTM! Well-named constants following consistent naming convention.The constant definitions are clear and follow a consistent naming pattern with appropriate default values.
1-1: Incorrect: Alias for@hyperse/logger-commonis already configuredBoth Vitest configs include the
@hyperse/logger-commonalias pointing topackages/logger-common/src/index.ts, so the module resolution setup is complete:
- packages/logger/vitest.config.ts
resolve: { alias: { '@hyperse/logger-common': getDirname( import.meta.url, '../logger-common/src/index.ts' ), // … } }- packages/logger-plugin-console/vitest.config.ts
resolve: { alias: { '@hyperse/logger-common': getDirname( import.meta.url, '../../logger-common/src/index.ts' ), // … } }And
packages/logger-common/src/index.tsexists with the appropriate exports. The original suggestion to add a missing alias is therefore not needed.Likely an incorrect or invalid review comment.
packages/logger/src/core/create-logger.ts (1)
5-15: LGTM! Clean factory function implementation.The
createLoggerfunction provides a clean and type-safe factory for creating logger instances. The generic constraints and default types are well-designed, making the API both flexible and easy to use.packages/logger/tests/error.spec.ts (1)
6-57: Excellent error handling test coverage.This test thoroughly validates the error handling mechanism by:
- Creating a plugin that consistently throws errors
- Verifying the error handler is called for each log level
- Checking that error messages are correctly propagated
The test structure and assertions are well-designed and provide good coverage of the error handling functionality.
packages/logger-common/lint-staged.config.mjs (1)
1-28: Standard lint-staged configuration looks good.The configuration follows the established patterns in the monorepo and includes appropriate settings for auto-fixing TypeScript and JavaScript files in the logger-common package.
packages/logger/tests/ctx.spec.ts (1)
6-89: Comprehensive context setup testing.The test suite effectively validates both synchronous and asynchronous context setup scenarios:
- Verifies that setup function results are correctly merged with initial context
- Tests both immediate and promise-based setup functions
- Uses proper mocking to inspect the merged context passed to plugins
The test coverage ensures the context merging mechanism works reliably in both sync and async scenarios.
packages/logger-plugin-console/src/helpers/helper-format-message.ts
Outdated
Show resolved
Hide resolved
packages/logger-plugin-console/src/helpers/helper-format-message.ts
Outdated
Show resolved
Hide resolved
packages/logger-plugin-console/src/helpers/helper-format-message.ts
Outdated
Show resolved
Hide resolved
packages/logger-plugin-console/src/helpers/helper-format-message.ts
Outdated
Show resolved
Hide resolved
| let output: any; | ||
| let i: number | string; | ||
| // handle case: array | ||
| if (input instanceof Array) { |
There was a problem hiding this comment.
Replace instanceof Array with Array.isArray() for better reliability.
Using instanceof Array can return false for arrays from other execution contexts (e.g., iframes). Array.isArray() is more reliable and is the recommended approach.
- if (input instanceof Array) {
+ if (Array.isArray(input)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 31-31: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🤖 Prompt for AI Agents
In packages/logger-common/src/helpers/helper-simple-deep-clone.ts at line 31,
replace the use of `input instanceof Array` with `Array.isArray(input)` to
reliably check if the input is an array, ensuring correct behavior across
different execution contexts like iframes.
| public build(): Pick< | ||
| BaseLogger<Context, Message>, | ||
| 'debug' | 'info' | 'warn' | 'error' | 'verbose' | ||
| > { | ||
| this.pipeline.use(async (_, next, error) => { | ||
| if (error) { | ||
| await executeFunction(this.errorHandling, error); | ||
| } | ||
| next(); | ||
| }); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding error handling earlier in the pipeline
The error handling middleware is only added when build() is called. If someone uses the logger without calling build(), errors will not be handled. Consider either:
- Adding error handling in the constructor
- Making the log methods throw if
build()hasn't been called - Documenting this requirement clearly
🤖 Prompt for AI Agents
In packages/logger/src/core/logger.ts around lines 77 to 88, the error handling
middleware is only added during the build() method, so errors won't be handled
if build() is not called. To fix this, move the addition of the error handling
middleware to the constructor so it is always present, or alternatively modify
the log methods to throw an error if build() hasn't been called yet, or clearly
document that build() must be called before using the logger.
| public debug(message: Message) { | ||
| this.pipeline.execute({ | ||
| ctx: this.ctx, | ||
| message: message, | ||
| priority: LogLevel.Debug, | ||
| }); | ||
| } | ||
|
|
||
| public info(message: Message) { | ||
| this.pipeline.execute({ | ||
| ctx: this.ctx, | ||
| message: message, | ||
| priority: LogLevel.Info, | ||
| }); | ||
| } | ||
|
|
||
| public warn(message: Message) { | ||
| this.pipeline.execute({ | ||
| ctx: this.ctx, | ||
| message: message, | ||
| priority: LogLevel.Warn, | ||
| }); | ||
| } | ||
|
|
||
| public error(message: Message) { | ||
| this.pipeline.execute({ | ||
| ctx: this.ctx, | ||
| message: message, | ||
| priority: LogLevel.Error, | ||
| }); | ||
| } | ||
|
|
||
| public verbose(message: Message) { | ||
| this.pipeline.execute({ | ||
| ctx: this.ctx, | ||
| message: message, | ||
| priority: LogLevel.Verbose, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Handle pipeline execution errors in log methods
The log methods call pipeline.execute() but don't handle potential errors. This could lead to silent failures or unhandled promise rejections.
Consider wrapping the pipeline execution in try-catch or returning the promise:
public debug(message: Message) {
- this.pipeline.execute({
+ return this.pipeline.execute({
ctx: this.ctx,
message: message,
priority: LogLevel.Debug,
+ }).catch((error) => {
+ if (this.errorHandling) {
+ this.errorHandling(error);
+ } else {
+ console.error('Logger pipeline error:', error);
+ }
});
}Apply similar changes to all log methods.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public debug(message: Message) { | |
| this.pipeline.execute({ | |
| ctx: this.ctx, | |
| message: message, | |
| priority: LogLevel.Debug, | |
| }); | |
| } | |
| public info(message: Message) { | |
| this.pipeline.execute({ | |
| ctx: this.ctx, | |
| message: message, | |
| priority: LogLevel.Info, | |
| }); | |
| } | |
| public warn(message: Message) { | |
| this.pipeline.execute({ | |
| ctx: this.ctx, | |
| message: message, | |
| priority: LogLevel.Warn, | |
| }); | |
| } | |
| public error(message: Message) { | |
| this.pipeline.execute({ | |
| ctx: this.ctx, | |
| message: message, | |
| priority: LogLevel.Error, | |
| }); | |
| } | |
| public verbose(message: Message) { | |
| this.pipeline.execute({ | |
| ctx: this.ctx, | |
| message: message, | |
| priority: LogLevel.Verbose, | |
| }); | |
| } | |
| public debug(message: Message) { | |
| return this.pipeline.execute({ | |
| ctx: this.ctx, | |
| message: message, | |
| priority: LogLevel.Debug, | |
| }).catch((error) => { | |
| if (this.errorHandling) { | |
| this.errorHandling(error); | |
| } else { | |
| console.error('Logger pipeline error:', error); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/logger/src/core/logger.ts between lines 90 and 128, the log methods
call pipeline.execute() without handling potential errors, which can cause
silent failures or unhandled promise rejections. To fix this, wrap each
pipeline.execute() call in a try-catch block to catch and handle any errors, or
return the promise and handle errors with .catch(). Apply this error handling
consistently across all log methods: debug, info, warn, error, and verbose.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/logger/src/core/logger.ts (1)
104-142: Handle pipeline execution errors in log methods.The log methods call
pipeline.execute()but don't handle potential errors, which could lead to unhandled promise rejections. This is the same issue flagged in previous reviews.Consider returning the promise or wrapping in try-catch:
public debug(message: Message) { - this.pipeline.execute({ + return this.pipeline.execute({ ctx: this.ctx, message: message, priority: LogLevel.Debug, + }).catch((error) => { + if (this.errorHandling) { + this.errorHandling(error); + } else { + console.error('Logger pipeline error:', error); + } }); }Apply similar changes to all log methods (info, warn, error, verbose).
🧹 Nitpick comments (5)
packages/logger-plugin-console/src/helpers/helper-format-stack.ts (1)
12-15: Consider making color configurable.The stack formatting is well-implemented but uses a hard-coded red color. Consider making this configurable through options.
The current implementation works well for the default case, but future flexibility would benefit from parameterized colors:
-export const formatStack = (stack: string) => +export const formatStack = (stack: string, color: Color[] = ['red']) => `\n${parseStack(stack) - .map((line) => ` ${terminalColor(['red'])(line)}`) + .map((line) => ` ${terminalColor(color)(line)}`) .join('\n')}`;packages/logger/tests/ctx.spec.ts (1)
2-2: Remove unused import to address static analysis warning.The
LoggerContexttype is imported but never used directly in the test file. The static analysis tool correctly flagged this.-import type { LoggerContext } from '../src/index.js';packages/logger-plugin-console/src/helpers/helper-format-time.ts (2)
9-12: Address unused remainder variable and potential logic issue.The
remaindervariable is updated on line 12 but never used again in the function. This suggests either:
- The remainder calculation is unnecessary, or
- The remainder should be passed to subsequent calls (though the current implementation recalculates from the original time difference)
If the remainder calculation is truly unused, remove it:
export const addUnitOfTime = ( prefix: string, time: Date, lastTime: Date, colorFn: (s: string) => string, unitValueInMilliseconds: number, unitName: string ) => { - let remainder = time.getTime() - lastTime.getTime(); + const timeDifference = time.getTime() - lastTime.getTime(); - const unitCount = Math.floor(remainder / unitValueInMilliseconds); + const unitCount = Math.floor(timeDifference / unitValueInMilliseconds); - remainder = remainder % unitValueInMilliseconds; return unitCount !== 0 ? colorFn(prefix + unitCount + unitName) + ' ' : unitValueInMilliseconds === 1 ? colorFn(prefix + '0') + ' ' : ''; };
28-90: Extract hardcoded time constants for better maintainability.The hardcoded millisecond values make the code less readable and maintainable. Consider extracting them into named constants.
+const TIME_UNITS = { + YEAR: 31536000000, // 365 * 24 * 60 * 60 * 1000 + MONTH: 2592000000, // 30 * 24 * 60 * 60 * 1000 + DAY: 86400000, // 24 * 60 * 60 * 1000 + HOUR: 3600000, // 60 * 60 * 1000 + MINUTE: 60000, // 60 * 1000 + SECOND: 1000, + MILLISECOND: 1 +} as const; export const formatTime = ( time: Date, lastTime: Date, decorateColorFn: (s: string) => string, color: (s: string) => string, prefix: string ) => { let formattedChangeInTime = ' '; // YEARS formattedChangeInTime += addUnitOfTime( prefix, time, lastTime, decorateColorFn, - 31536000000, + TIME_UNITS.YEAR, 'y' ); // Apply similar changes for other time units...packages/logger-plugin-console/src/helpers/helper-format-message.ts (1)
12-145: Significant improvements made to the implementation!The formatting logic is now much more complete compared to the previous review comments:
- ✅ Date and timestamp formatting fully implemented (lines 48-82)
- ✅ Level name formatting implemented (lines 84-92)
- ✅ Context formatting for prefix, logger name, and plugin name (lines 94-123)
- ✅ Arrow, message name, message, and stack formatting (lines 125-144)
However, some unused variables still need to be addressed to fix linting warnings.
Address the remaining unused variables by prefixing with underscore:
const { showLoggerName, capitalizeLoggerName, showPluginName, capitalizePluginName, showPrefix, showLevelName, capitalizeLevelName, showDate, showTimestamp, use24HourClock, + showRelativeTimestamp: _showRelativeTimestamp, + showMonthBeforeDay: _showMonthBeforeDay, + showTimestampRelativeToLastLog: _showTimestampRelativeToLastLog, showArrow, noColor, prefixColor, loggerNameColor, pluginNameColor, } = options;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
packages/logger-plugin-console/package.json(2 hunks)packages/logger-plugin-console/src/constant.ts(1 hunks)packages/logger-plugin-console/src/create-console-plugin.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-assert-message.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-color-applier.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-message.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-stack.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-time.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-is-loggable.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-merge-optons.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-normalize-level.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-str-pad.ts(1 hunks)packages/logger-plugin-console/src/index.ts(1 hunks)packages/logger-plugin-console/src/types/index.ts(1 hunks)packages/logger-plugin-console/src/types/type-options.ts(1 hunks)packages/logger-plugin-console/src/types/type-plugin.ts(1 hunks)packages/logger-plugin-console/tests/console.ts(1 hunks)packages/logger/src/constant/constant.ts(1 hunks)packages/logger/src/constant/index.ts(1 hunks)packages/logger/src/constant/log-level.ts(1 hunks)packages/logger/src/core/create-logger.ts(1 hunks)packages/logger/src/core/logger.ts(1 hunks)packages/logger/src/helpers/helper-merge-options.ts(1 hunks)packages/logger/src/helpers/helper-simple-deep-clone.ts(1 hunks)packages/logger/src/helpers/index.ts(1 hunks)packages/logger/src/index.ts(1 hunks)packages/logger/src/plugin/define-plugin.ts(1 hunks)packages/logger/src/plugin/index.ts(1 hunks)packages/logger/src/types/index.ts(1 hunks)packages/logger/src/types/type-logger-plugin.ts(1 hunks)packages/logger/src/types/type-logger.ts(1 hunks)packages/logger/src/types/type-message.ts(1 hunks)packages/logger/src/types/type-partial-deep.ts(1 hunks)packages/logger/tests/ctx.spec.ts(1 hunks)packages/logger/tests/error.spec.ts(1 hunks)packages/logger/tests/logger.spec.ts(1 hunks)packages/logger/tests/test-utils.ts(1 hunks)packages/logger/tsconfig.json(1 hunks)packages/logger/vitest.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/logger/src/constant/log-level.ts
- packages/logger-plugin-console/src/types/index.ts
- packages/logger-plugin-console/src/index.ts
- packages/logger/src/types/type-partial-deep.ts
- packages/logger/src/types/type-message.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/logger/vitest.config.ts
- packages/logger/src/plugin/index.ts
- packages/logger-plugin-console/package.json
- packages/logger/src/constant/index.ts
- packages/logger/tsconfig.json
- packages/logger-plugin-console/src/helpers/helper-merge-optons.ts
- packages/logger/src/plugin/define-plugin.ts
- packages/logger/tests/error.spec.ts
- packages/logger-plugin-console/src/types/type-options.ts
- packages/logger-plugin-console/tests/console.ts
- packages/logger/src/constant/constant.ts
- packages/logger/src/index.ts
- packages/logger/tests/logger.spec.ts
- packages/logger-plugin-console/src/constant.ts
- packages/logger/tests/test-utils.ts
- packages/logger/src/types/type-logger.ts
🧰 Additional context used
🧬 Code Graph Analysis (13)
packages/logger-plugin-console/src/helpers/helper-format-stack.ts (1)
packages/logger-plugin-console/src/helpers/helper-color-applier.ts (1)
terminalColor(10-22)
packages/logger-plugin-console/src/helpers/helper-is-loggable.ts (2)
packages/logger/src/types/type-logger.ts (1)
LoggerContext(5-8)packages/logger-plugin-console/src/types/type-plugin.ts (1)
ConsolePluginContext(4-4)
packages/logger-plugin-console/src/helpers/helper-normalize-level.ts (3)
packages/logger-plugin-console/src/types/type-options.ts (1)
ConsoleOptions(4-123)packages/logger-plugin-console/src/types/type-plugin.ts (1)
LevelData(10-25)packages/logger-plugin-console/src/constant.ts (1)
defaultLevelColor(5-11)
packages/logger-plugin-console/src/helpers/helper-assert-message.ts (1)
packages/logger/src/types/type-message.ts (1)
LoggerMessageObject(1-6)
packages/logger-plugin-console/src/create-console-plugin.ts (6)
packages/logger-plugin-console/src/helpers/helper-merge-optons.ts (1)
mergeConsoleOptions(5-9)packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger-plugin-console/src/types/type-plugin.ts (2)
ConsolePluginContext(4-4)ConsolePluginMessage(6-8)packages/logger-plugin-console/src/helpers/helper-is-loggable.ts (1)
isLoggable(4-9)packages/logger-plugin-console/src/helpers/helper-assert-message.ts (1)
assertMessage(3-14)packages/logger-plugin-console/src/helpers/helper-format-message.ts (1)
formatMessage(12-145)
packages/logger-plugin-console/src/types/type-plugin.ts (2)
packages/logger/src/types/type-message.ts (1)
LoggerMessageObject(1-6)packages/logger-plugin-console/src/types/type-color.ts (1)
Color(3-6)
packages/logger/src/core/create-logger.ts (4)
packages/logger/src/types/type-message.ts (1)
LoggerMessage(8-8)packages/logger/src/types/type-logger.ts (1)
LoggerContext(5-8)packages/logger/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger/src/core/logger.ts (2)
error(128-134)Logger(28-143)
packages/logger-plugin-console/src/helpers/helper-color-applier.ts (1)
packages/logger-plugin-console/src/types/type-color.ts (1)
Color(3-6)
packages/logger/src/helpers/helper-merge-options.ts (2)
packages/logger/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger/src/helpers/helper-simple-deep-clone.ts (3)
simpleDeepClone(21-51)isObject(5-7)isClassInstance(9-15)
packages/logger-plugin-console/src/helpers/helper-format-message.ts (7)
packages/logger/src/types/type-logger-plugin.ts (1)
LoggerPluginContext(11-14)packages/logger-plugin-console/src/types/type-plugin.ts (2)
ConsolePluginContext(4-4)ConsolePluginMessage(6-8)packages/logger-plugin-console/src/types/type-options.ts (1)
ConsoleOptions(4-123)packages/logger-plugin-console/src/helpers/helper-normalize-level.ts (1)
normalizeLevelData(6-18)packages/logger-plugin-console/src/helpers/helper-color-applier.ts (2)
getColorApplier(24-37)terminalColor(10-22)packages/logger-plugin-console/src/helpers/helper-str-pad.ts (1)
strTimePad(47-49)packages/logger-plugin-console/src/helpers/helper-format-stack.ts (1)
formatStack(12-15)
packages/logger/src/core/logger.ts (8)
packages/logger/src/types/type-message.ts (1)
LoggerMessage(8-8)packages/logger/src/types/type-logger.ts (2)
BaseLogger(15-29)LoggerContext(5-8)packages/logger/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger/src/constant/constant.ts (2)
defaultLoggerName(3-3)defaultLogLevel(7-7)packages/logger/src/helpers/helper-merge-options.ts (1)
mergeOptions(34-75)packages/logger/src/helpers/helper-execute-fun.ts (1)
executeFunction(7-16)packages/logger/src/types/type-logger-plugin.ts (1)
LoggerPlugin(16-30)packages/logger/src/helpers/helper-simple-deep-clone.ts (1)
simpleDeepClone(21-51)
packages/logger/src/types/type-logger-plugin.ts (2)
packages/logger/src/types/type-logger.ts (1)
LoggerContext(5-8)packages/logger/src/types/type-message.ts (1)
LoggerMessage(8-8)
packages/logger/tests/ctx.spec.ts (3)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger/src/core/create-logger.ts (1)
createLogger(5-17)packages/logger/tests/test-utils.ts (1)
sleep(28-32)
🪛 Biome (1.9.4)
packages/logger/src/helpers/helper-simple-deep-clone.ts
[error] 46-46: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
[error] 31-31: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🪛 GitHub Check: test (18.x)
packages/logger/tests/ctx.spec.ts
[warning] 2-2:
'LoggerContext' is defined but never used. Allowed unused vars must match /^_/u
🔇 Additional comments (23)
packages/logger/src/helpers/index.ts (1)
1-2: LGTM! Clean barrel export pattern.The re-export structure is clean and follows good modular design principles. The
.jsfile extensions in the imports suggest proper ES module configuration for TypeScript compilation.packages/logger/src/types/index.ts (1)
1-4: LGTM! Well-organized type definitions.The barrel export pattern for types is consistent with the helpers module and provides good organization for the logger type system. The four type modules suggest comprehensive coverage of the logger's type requirements.
packages/logger-plugin-console/src/helpers/helper-is-loggable.ts (1)
4-9: LGTM! Clean and correct log level filtering.The implementation is straightforward and correct. The
>=comparison assumes that higher numerical values represent more verbose log levels, which is a standard convention.The function properly leverages TypeScript types and provides a clear interface for log level filtering.
packages/logger-plugin-console/src/helpers/helper-normalize-level.ts (1)
6-18: Clean and well-structured helper function.The implementation correctly handles color fallbacks using nullish coalescing and leverages enum reverse mapping to convert numeric log levels to string names. The type signatures are appropriate and the logic is straightforward.
packages/logger/src/core/create-logger.ts (1)
5-17: Well-designed factory function with appropriate generics.The function provides a clean API for creating logger instances with proper type safety. The generic constraints and optional setup/error handling configuration make it flexible while maintaining type safety.
packages/logger-plugin-console/src/create-console-plugin.ts (1)
12-48: Excellent use of pipe-based architecture for plugin execution.The sequential pipe flow provides a clean separation of concerns:
- Log level filtering
- Message normalization
- Message formatting
- Console output
This approach makes the plugin logic easy to follow and maintain. The early exit pattern with
exitPipewhen priority is too low is also well-implemented.packages/logger-plugin-console/src/types/type-plugin.ts (1)
4-25: Well-defined types with clear documentation.The type definitions are appropriate for the console plugin:
ConsolePluginContextas an extensible object type provides flexibilityConsolePluginMessagelogically extends the base message with optional timestampLevelDatastructure matches the normalized level data requirements- JSDoc comments clearly explain each property's purpose
packages/logger/tests/ctx.spec.ts (2)
7-44: Comprehensive test coverage for context setup scenarios.The test effectively verifies both synchronous and asynchronous context setup with proper assertions on the merged context. The use of mock plugins to intercept and verify the context is a good testing approach.
46-91: Good async test coverage with proper timing.The asynchronous setup test properly handles the Promise-based setup function and verifies the context merging after the async operation completes. The test structure mirrors the synchronous test effectively.
packages/logger-plugin-console/src/helpers/helper-color-applier.ts (2)
10-22: LGTM: Clean color application logic.The
terminalColorfunction handles edge cases well (no colors, noColor flag) and applies colors sequentially as expected. The early return with an identity function for disabled colors is efficient.
24-37: LGTM: Well-structured color filtering.The
getColorApplierfunction clearly separates decorations (strikethrough, underline) from regular colors. The filtering logic is straightforward and the function composition withterminalColoris clean.packages/logger/src/helpers/helper-merge-options.ts (2)
8-10: LGTM: Clear helper function.The
needMergehelper clearly determines when a value should be merged based on themergeUndefinedflag.
34-75: LGTM: Robust deep merge implementation.The
mergeOptionsfunction handles complex scenarios well:
- Clones target at depth 0 to prevent mutation
- Treats arrays as atomic values (documented behavior)
- Properly handles class instances vs plain objects
- Uses recursive approach with depth tracking
The implementation correctly balances flexibility with safety.
packages/logger-plugin-console/src/helpers/helper-str-pad.ts (3)
6-29: LGTM: Robust repeat implementation with excellent error handling.The custom
repeatfunction handles all edge cases well:
- Validates non-negative count
- Prevents infinite repeat count
- Checks for string size overflow
- Uses bitwise operation for efficient integer conversion
The implementation is thorough and safe.
37-45: LGTM: Clean padding logic.The
strPadfunction correctly handles both left and right padding using the sign of the length parameter. The logic is clear and efficient.
47-49: LGTM: Concise time padding utility.The
strTimePadfunction is a well-focused wrapper for zero-padding time values to at least 2 characters.packages/logger/src/helpers/helper-simple-deep-clone.ts (3)
5-7: LGTM: Clean object type guard.The
isObjectfunction correctly identifies non-array objects with a clear implementation.
9-15: LGTM: Robust class instance detection.The
isClassInstancefunction properly handles edge cases like null-prototype objects and correctly identifies class instances.
21-51: LGTM: Well-structured deep cloning logic (with fixes needed above).The
simpleDeepClonefunction has solid logic for handling different data types:
- Handles primitives and null correctly
- Recursively clones arrays and plain objects
- Preserves class instances as-is
- Uses proper type checking utilities
Once the static analysis issues above are addressed, this will be a robust cloning utility.
packages/logger/src/types/type-logger-plugin.ts (2)
11-14: LGTM: Clean context extension.The
LoggerPluginContexttype cleanly extends the baseLoggerContextwith apluginNameproperty, maintaining type safety while adding plugin-specific context.
16-30: LGTM: Well-designed plugin interface.The
LoggerPlugininterface provides excellent flexibility:
- Generic types for both Context and Message allow customization
- The execute method receives all necessary pipeline utilities
- Supports both synchronous and asynchronous plugin execution
- Clean separation of concerns with the options object pattern
This design enables powerful and flexible plugin implementations.
packages/logger/src/core/logger.ts (2)
61-71: LGTM! Async handling has been properly implemented.The
useSetupCtxmethod now correctly awaits thenext()call, addressing the previous review feedback about async flow control.
73-89: LGTM! Plugin execution and pipeline flow properly implemented.Both
plugin.execute()andnext()are now properly awaited, ensuring correct asynchronous pipeline execution. This addresses the previous review concerns about async handling.
| output = {}; | ||
| for (i in input) { | ||
| // eslint-disable-next-line no-prototype-builtins | ||
| if (input.hasOwnProperty(i)) { |
There was a problem hiding this comment.
Replace hasOwnProperty with Object.hasOwn().
Static analysis correctly identified this issue. Object.hasOwn() is the modern, safer alternative to Object.prototype.hasOwnProperty.call().
- if (input.hasOwnProperty(i)) {
+ if (Object.hasOwn(input, i)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (input.hasOwnProperty(i)) { | |
| if (Object.hasOwn(input, i)) { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 46-46: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.
(lint/suspicious/noPrototypeBuiltins)
🤖 Prompt for AI Agents
In packages/logger/src/helpers/helper-simple-deep-clone.ts at line 46, replace
the use of input.hasOwnProperty(i) with Object.hasOwn(input, i) to use the
modern and safer method for checking own properties.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
packages/logger/tests/logger.spec.ts (4)
56-56: Replace arbitrary sleep with proper async handlingThe fixed sleep duration can make tests flaky. Since past reviews identified this issue and it remains unaddressed, consider implementing a more reliable approach to wait for async operations to complete.
99-99: Replace arbitrary sleep with proper async handlingSame issue as line 56 - using fixed sleep durations can make tests flaky and unreliable.
169-169: Replace arbitrary sleep with proper async handlingSame issue as lines 56 and 99 - fixed sleep durations should be replaced with proper async handling mechanisms.
30-187: Add negative test cases for error scenariosThe test suite only covers happy path scenarios. As noted in previous reviews, consider adding tests for error cases to improve coverage and robustness.
packages/logger/src/core/logger.ts (3)
61-71: Use standard method syntax for private methodsThe
useSetupCtxmethod is still using arrow function syntax despite being marked as addressed in previous reviews. For consistency and better TypeScript support, use standard method syntax for private methods.
106-113: Consider adding error handling earlier in the pipelineError handling middleware is only added when
build()is called. If someone uses the logger without callingbuild(), errors will not be handled. This issue was raised in previous reviews and remains unaddressed.
119-173: Handle pipeline execution errors in log methodsThe log methods call
pipeline.execute()but don't handle potential errors, which could lead to silent failures or unhandled promise rejections. This issue was identified in previous reviews and remains unaddressed.
🧹 Nitpick comments (1)
packages/logger/tests/merge-options/mergeOptions.spec.ts (1)
29-29: Fix typo in test description.There's a typo in the test description.
- it('Should correct merge differenct type fields', () => { + it('Should correctly merge different type fields', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
packages/logger-plugin-console/package.json(2 hunks)packages/logger-plugin-console/src/constant.ts(1 hunks)packages/logger-plugin-console/src/create-console-plugin.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-assert-message.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-message.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-format-stack.ts(1 hunks)packages/logger-plugin-console/src/helpers/helper-is-loggable.ts(1 hunks)packages/logger-plugin-console/src/types/type-options.ts(1 hunks)packages/logger-plugin-console/tests/console-plugin.spec.ts(1 hunks)packages/logger-plugin-console/tests/console.ts(1 hunks)packages/logger-plugin-console/tests/index.test.ts(0 hunks)packages/logger-plugin-console/tests/test-utils.ts(1 hunks)packages/logger-plugin-sentry/package.json(1 hunks)packages/logger/package.json(1 hunks)packages/logger/src/core/logger.ts(1 hunks)packages/logger/src/helpers/helper-simple-deep-clone.ts(1 hunks)packages/logger/src/types/type-logger-plugin.ts(1 hunks)packages/logger/src/types/type-logger.ts(1 hunks)packages/logger/tests/ctx.spec.ts(1 hunks)packages/logger/tests/error.spec.ts(1 hunks)packages/logger/tests/logger.spec.ts(1 hunks)packages/logger/tests/merge-options/mergeOptions.spec.ts(1 hunks)packages/logger/tests/merge-options/simpleDeepClone.spec.ts(1 hunks)packages/logger/tests/test-utils.ts(1 hunks)website/next.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/logger-plugin-console/tests/index.test.ts
✅ Files skipped from review due to trivial changes (4)
- website/next.config.ts
- packages/logger-plugin-sentry/package.json
- packages/logger/package.json
- packages/logger-plugin-console/src/constant.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- packages/logger-plugin-console/package.json
- packages/logger-plugin-console/src/helpers/helper-is-loggable.ts
- packages/logger-plugin-console/src/types/type-options.ts
- packages/logger-plugin-console/src/helpers/helper-assert-message.ts
- packages/logger-plugin-console/src/create-console-plugin.ts
- packages/logger/tests/error.spec.ts
- packages/logger/tests/ctx.spec.ts
- packages/logger/tests/test-utils.ts
- packages/logger-plugin-console/src/helpers/helper-format-stack.ts
- packages/logger/src/helpers/helper-simple-deep-clone.ts
- packages/logger-plugin-console/src/helpers/helper-format-message.ts
- packages/logger-plugin-console/tests/console.ts
- packages/logger/src/types/type-logger-plugin.ts
🧰 Additional context used
🧬 Code Graph Analysis (7)
packages/logger-plugin-console/tests/test-utils.ts (1)
packages/logger/tests/test-utils.ts (1)
sleep(28-32)
packages/logger/tests/merge-options/simpleDeepClone.spec.ts (1)
packages/logger/src/helpers/helper-simple-deep-clone.ts (1)
simpleDeepClone(21-50)
packages/logger/tests/merge-options/mergeOptions.spec.ts (1)
packages/logger/src/helpers/helper-merge-options.ts (1)
mergeOptions(34-75)
packages/logger-plugin-console/tests/console-plugin.spec.ts (3)
packages/logger/src/core/create-logger.ts (1)
createLogger(5-17)packages/logger-plugin-console/src/create-console-plugin.ts (1)
createConsolePlugin(12-45)packages/logger-plugin-console/tests/test-utils.ts (1)
sleep(1-5)
packages/logger/src/core/logger.ts (8)
packages/logger/src/types/type-message.ts (1)
LoggerMessage(8-8)packages/logger/src/types/type-logger.ts (2)
BaseLogger(15-29)LoggerContext(5-8)packages/logger/src/types/type-partial-deep.ts (1)
DeepPartial(11-11)packages/logger/src/constant/constant.ts (2)
defaultLoggerName(3-3)defaultLogLevel(7-7)packages/logger/src/helpers/helper-merge-options.ts (1)
mergeOptions(34-75)packages/logger/src/helpers/helper-execute-fun.ts (1)
executeFunction(7-16)packages/logger/src/types/type-logger-plugin.ts (1)
LoggerPlugin(16-30)packages/logger/src/helpers/helper-simple-deep-clone.ts (1)
simpleDeepClone(21-50)
packages/logger/tests/logger.spec.ts (5)
packages/logger/src/plugin/define-plugin.ts (1)
definePlugin(12-14)packages/logger/tests/test-utils.ts (2)
setUpForTest(4-26)sleep(28-32)packages/logger/src/core/create-logger.ts (1)
createLogger(5-17)packages/logger-plugin-console/tests/test-utils.ts (1)
sleep(1-5)packages/logger/src/types/type-logger.ts (1)
LoggerContext(5-8)
packages/logger/src/types/type-logger.ts (2)
packages/logger/src/types/type-message.ts (1)
LoggerMessage(8-8)packages/logger/src/types/type-logger-plugin.ts (1)
LoggerPlugin(16-30)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: deploy (20.x)
🔇 Additional comments (4)
packages/logger/tests/merge-options/simpleDeepClone.spec.ts (1)
1-82: Excellent test coverage for the deep clone utility.This test suite provides comprehensive coverage of the
simpleDeepClonefunction with well-structured test cases that verify:
- Deep cloning of nested objects and arrays
- Immutability (no shared references)
- Proper handling of primitives and class instances
- Edge cases like class instances within collections
The test names are descriptive and the assertions appropriately verify both equality and reference independence. This is exactly the kind of thorough testing needed for utility functions.
packages/logger/tests/merge-options/mergeOptions.spec.ts (1)
3-313: Excellent comprehensive test suite for options merging.This test suite provides outstanding coverage of the
mergeOptionsfunction with scenarios including:
- Immutability verification (no mutation of original objects)
- Deep and shallow merging
- Handling of different data types (objects, arrays, functions, class instances)
- Undefined value merging with the
mergeUndefinedflag- Complex nested object scenarios
- Edge cases like array replacement vs merging
The test cases are well-structured and the assertions properly verify both the merged results and immutability guarantees. This thorough testing will help ensure the reliability of the core merging functionality used throughout the logger system.
packages/logger-plugin-console/tests/console-plugin.spec.ts (1)
5-150: Well-structured test suite with comprehensive coverage.This test suite effectively validates the console plugin functionality with:
- Proper test isolation using
beforeEach/afterEachfor mock setup/cleanup- Coverage of key scenarios: disabled plugin, various log levels, string/object messages, error stack traces
- Appropriate use of
sleep()to handle asynchronous logging operations- Good assertion patterns using regex matching for output verification
- Clear test naming that describes expected behavior
The mock strategy for
console.logis appropriate and the test scenarios align well with the plugin's documented functionality. This provides confidence in the console plugin's correctness.packages/logger/src/types/type-logger.ts (1)
1-29: Well-designed type definitions with proper generic constraints.These type definitions provide a solid foundation for the logger system with:
LoggerContext: Properly extends generic Context while enforcing requirednameandthresholdLevelpropertiesLoggerPluginList: Clean array type for plugin collections with appropriate generic constraintsBaseLogger: Well-designed interface supporting fluent API pattern withuse().build()chaining and comprehensive logging methodsThe generic type parameters are properly constrained (
Context extends object,Message extends LoggerMessage) and the interface design promotes type safety while maintaining flexibility. Thebuild()method's return type appropriately restricts to only the logging methods, which is a good design choice for the built logger instance.
| export async function sleep(ms = 1000) { | ||
| return await new Promise<void>((resolver) => { | ||
| setTimeout(resolver, ms); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication by consolidating test utilities.
This sleep function is identical to the one in packages/logger/tests/test-utils.ts (lines 28-32). Having duplicate utility functions across test files violates the DRY principle and creates maintenance overhead.
Consider one of these approaches:
- Preferred: Create a shared test utilities package that both test suites can import
- Alternative: Move this utility to the logger package and export it for use by plugin tests
-export async function sleep(ms = 1000) {
- return await new Promise<void>((resolver) => {
- setTimeout(resolver, ms);
- });
-}Then import from the shared location:
import { sleep } from '@hyperse/logger/tests/test-utils';🤖 Prompt for AI Agents
In packages/logger-plugin-console/tests/test-utils.ts lines 1 to 5, the sleep
function duplicates the identical utility in packages/logger/tests/test-utils.ts
lines 28 to 32. To fix this, remove the local sleep function and instead import
sleep from a shared test utilities location, preferably by creating a shared
test utilities package or moving the utility to the logger package and exporting
it. Update the import statement to import { sleep } from
'@hyperse/logger/tests/test-utils' to eliminate duplication and centralize
maintenance.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores