-
Notifications
You must be signed in to change notification settings - Fork 324
refactor(utils): [grid,tree] rename log to logger #2826
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
Conversation
WalkthroughThis pull request introduces a comprehensive update to the logging mechanism across multiple packages in the project. The primary change involves replacing the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Walkthrough此PR重命名了utils模块中的导出名称,从log改为logger,并确保所有logger只使用warn和error方法。注意,packages/mobile目录下的文件没有修改。 Changes
|
@@ -555,7 +555,7 @@ export const handleStart = | |||
state, | |||
vm | |||
}: Pick<IFileUploadRenderlessParams, 'api' | 'constants' | 'props' | 'state' | 'vm'>) => | |||
(rawFiles: IFileUploadFile[], updateId: string, reUpload: boolean = false) => { | |||
(rawFiles: IFileUploadFile[], updateId: string, reUpload = 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.
The default parameter syntax has been simplified from reUpload: boolean = false
to reUpload = false
. Ensure that this change does not affect the expected behavior of the function.
@@ -900,7 +900,7 @@ export const abort = | |||
|
|||
export const abortDownload = | |||
({ state }: Pick<IFileUploadRenderlessParams, 'state'>) => | |||
(file: IFileUploadFile, batch: boolean = false) => { | |||
(file: IFileUploadFile, batch = 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.
The default parameter syntax has been simplified from batch: boolean = false
to batch = false
. Ensure that this change does not affect the expected behavior of the function.
@@ -2263,7 +2263,7 @@ export const getToken = | |||
|
|||
export const previewFile = | |||
({ api, props }: Pick<IFileUploadRenderlessParams, 'api' | 'props'>) => | |||
(file: IFileUploadFile, open: boolean = false) => { | |||
(file: IFileUploadFile, open = 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.
The default parameter syntax has been simplified from open: boolean = false
to open = false
. Ensure that this change does not affect the expected behavior of the function.
WalkthroughThis PR renames the export names in the utils module from log to logger and ensures that all loggers only use the warn and error methods. Note that the files in the packages/mobile directory have not been modified. Changes
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
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: 0
🧹 Nitpick comments (4)
packages/utils/src/logger/index.ts (2)
3-3
: Consider stricter typing for window object.Using
any
type bypasses TypeScript's type checking. Consider using a more specific type or interface that defines the expected shape of the window object.-const _win: any = getWindow() +interface WindowWithConsole { + console: Console; +} +const _win = getWindow() as WindowWithConsole
4-5
: Improve documentation and error handling.
- Consider translating the Chinese comment to English for consistency.
- Add error handling for environments where window/console might not be available.
-/** 使用 logger.xxx 代替 window.console.xxx, 避免语法警告 */ +/** Use logger.xxx instead of window.console.xxx to avoid syntax warnings in strict mode */ -export const logger = _win.console as Console +export const logger = (_win?.console || console) as Consolepackages/vue/src/grid/src/tools/logger.ts (1)
Line range hint
4-11
: Add type safety for logger methods.The
type
parameter should be restricted to valid logger methods to prevent runtime errors.-const outLog = (type) => (message, detail) => { +type LogLevel = 'warn' | 'error'; +const outLog = (type: LogLevel) => (message: string, detail?: string) => {packages/renderless/src/currency/index.ts (1)
79-79
: Enhance warning message with more context.The current warning message is generic. Consider including more details about the data error to help with debugging.
-logger.warn('data error. set it to the default value.') +logger.warn(`Currency data error: Missing ${valueField} and ${textField}. Falling back to default value: ${props.currency}`)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/renderless/src/common/validate/util.ts
(3 hunks)packages/renderless/src/currency/index.ts
(2 hunks)packages/renderless/src/file-upload/index.ts
(8 hunks)packages/renderless/src/tree/index.ts
(2 hunks)packages/renderless/src/user/index.ts
(2 hunks)packages/utils/src/index.ts
(1 hunks)packages/utils/src/log/index.ts
(0 hunks)packages/utils/src/logger/index.ts
(1 hunks)packages/vue-common/package.json
(1 hunks)packages/vue-common/src/breakpoint.ts
(1 hunks)packages/vue/src/grid/src/tools/logger.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/utils/src/log/index.ts
✅ Files skipped from review due to trivial changes (3)
- packages/utils/src/index.ts
- packages/vue-common/src/breakpoint.ts
- packages/renderless/src/tree/index.ts
🔇 Additional comments (4)
packages/renderless/src/common/validate/util.ts (1)
211-211
: LGTM! Error handling implementation is robust.The code correctly distinguishes between validation errors (which have errors and fields properties) and actual errors that should be logged.
Also applies to: 247-247
packages/renderless/src/user/index.ts (1)
17-17
: LGTM! Import and usage of logger updated correctly.The changes correctly implement the new logging pattern:
- Import statement updated from
log
tologger
- Usage updated from
log.logger.warn
tologger.warn
Also applies to: 142-142
packages/renderless/src/file-upload/index.ts (1)
33-33
: LGTM! Logger import and usage updated consistently.All instances of the logger have been correctly updated:
- Import statement changed from
log
tologger
- Four instances of
log.logger.warn
updated tologger.warn
- Warning message formats preserved
Also applies to: 1688-1688, 1697-1697, 1935-1935, 1945-1945
packages/vue-common/package.json (1)
34-34
: LGTM! Required dependency added correctly.The @opentiny/utils dependency has been properly added with workspace resolution, which is required for the logger functionality.
PR
重新指定utils的导出名称, 从log改为logger
确保所有logger只使用 warn ,error的方法
packages/mobile内的没有修改。
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Refactor
log
tologger
Dependency Updates
@opentiny/utils
as a workspace dependency invue-common
packageChores