-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: mute debug log warnings #259
Conversation
Walkthrough此次更改主要集中在 Changes
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range comments (1)
packages/core/src/loggers.ts (1)
Line range hint
62-93
: 建议优化日志记录逻辑的实现当前实现正确但可以更简洁。建议:
- 使用可选链操作符简化日志调用
- 提取日志前缀为常量
- _createExternalLog = - (name: string) => - (...args: any[]) => { - this.once('debugReady', () => { - if (!this.isFaked) { - // @ts-ignore - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - this[name]?.(...args); - } - }); - // eslint-disable-next-line @typescript-eslint/no-unsafe-argument - return this._externalLogger?.log?.(`${name} >>> `, ...args); - }; + private readonly LOG_PREFIX = ' >>> '; + + _createExternalLog = (name: string) => (...args: any[]) => { + this.once('debugReady', () => { + !this.isFaked && this[name]?.(...args); + }); + return this._externalLogger?.log?.(name + this.LOG_PREFIX, ...args); + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
packages/core/src/loggers.ts
(6 hunks)
🔇 Additional comments (1)
packages/core/src/loggers.ts (1)
153-153
: 导出语句重排序符合预期
按字母顺序重新排列的导出语句提高了可读性。
@@ -95,11 +100,14 @@ class AppDebugLogger extends FakeDebugLogger { | |||
// TODO createDebugSync | |||
void createDebugAsync().then((debug) => { | |||
this._debug = debug; | |||
this.isFaked = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
消除重复的 isFaked 初始化
isFaked
属性在两处被初始化,这可能导致混淆。建议只在一处设置初始值。
constructor() {
super();
void createDebugAsync().then((debug) => {
this._debug = debug;
- this.isFaked = false;
this.initDebugInstances();
this.emit('debugReady');
});
}
- isFaked = false;
Also applies to: 109-110
@@ -41,6 +41,7 @@ class FakeDebugLogger extends CrossEventEmitter implements IDebugLogger { | |||
constructor() { | |||
super(); | |||
this.initExternalLogInstances(); | |||
this.setMaxListeners(9999); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
警告:设置过高的最大监听器数量可能掩盖内存泄漏问题
将 setMaxListeners
设置为 9999 是一个危险的做法。建议:
- 评估实际需要的监听器数量
- 检查是否存在未正确移除的监听器
- 考虑使用更合理的限制值
- this.setMaxListeners(9999);
+ this.setMaxListeners(50); // 设置一个合理的限制值
📝 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.
this.setMaxListeners(9999); | |
this.setMaxListeners(50); // 设置一个合理的限制值 |
Summary by CodeRabbit
新功能
isFaked
属性,以增强日志记录器的控制流。改进