-
Notifications
You must be signed in to change notification settings - Fork 140
refactor: Upgrade utils and replace useMergedState #400
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
base: master
Are you sure you want to change the base?
refactor: Upgrade utils and replace useMergedState #400
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough对 Collapse 组件的内部状态管理进行了重构:由 useMergedState 切换为 useControlledState 与 useEvent 的受控/非受控模式;统一通过 getActiveKeysArray 派生 activeKey,并使用 triggerActiveKey 进行状态更新与 onChange 调用。并将 @rc-component/util 依赖从 ^1.0.1 升级到 ^1.3.0。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as 用户
participant C as Collapse
participant S as useControlledState
participant E as useEvent (triggerActiveKey)
participant P as 父组件 onChange
U->>C: 点击 Panel Header
C->>C: onItemClick 计算 nextKeys(手风琴/多选)
C->>E: 调用 triggerActiveKey(nextKeys)
E->>C: 派生 currentKeys = getActiveKeysArray(internal)
alt nextKeys 与 currentKeys 不同
E->>S: setInternal(nextKeys)
E-->>P: 触发 onChange(nextKeys)
else 无变化
E-->>C: 不做状态更新/不触发 onChange
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Summary of Changes
Hello @EmilyyyLiu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
此 PR 主要进行了一项重构,旨在更新项目所依赖的工具库版本,并替换组件内部的状态管理 Hook。通过升级 @rc-component/util
并迁移到 useControlledState
,提高了组件对受控和非受控状态处理的灵活性和一致性,同时优化了事件触发机制。
Highlights
- 依赖升级: 将
package.json
中的@rc-component/util
版本从^1.0.1
升级到^1.3.0
。 - 替换状态管理 Hook: 在
src/Collapse.tsx
中,将useMergedState
替换为useControlledState
和useEvent
。 - 重构
activeKey
逻辑: 调整Collapse
组件中activeKey
的状态管理逻辑,以适应useControlledState
的使用,并优化onItemClick
的触发方式。
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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.
Code Review
本次 PR 主要是将 @rc-component/util
依赖从 ^1.0.1
升级到 ^1.3.0
,并用 useControlledState
替换了已废弃的 useMergedState
。整体重构方向是正确的,但有两处可以改进:一处是 triggerActiveKey
中对数组的比较逻辑有误,可能导致不必要的 onChange
事件触发;另一处是 onItemClick
的逻辑虽然正确,但可读性不佳,建议重构以提高代码的可维护性。
src/Collapse.tsx
Outdated
if (next !== activeKey) { | ||
onChange?.(next); | ||
} |
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.
在 triggerActiveKey
函数中,使用 next !== activeKey
来判断数组是否发生变化是不正确的。因为 next
和 activeKey
总是不同的数组实例,即使它们的内容完全相同,这个条件也始终为 true
。这会导致 onChange
回调在每次点击时都会被不必要地触发,可能引起父组件的额外渲染。之前的 useMergedState
内部使用了浅比较来避免此问题,建议此处也实现一个浅比较逻辑。
if (
next.length !== activeKey.length ||
next.some((key, index) => key !== activeKey[index])
) {
onChange?.(next);
}
const onItemClick = (key: React.Key) => { | ||
let keys = [...activeKey, key]; | ||
if (accordion) { | ||
keys = activeKey[0] === key ? [] : [key]; | ||
} | ||
const index = activeKey.indexOf(key); | ||
const isActive = index > -1; | ||
if (isActive) { | ||
keys = activeKey.filter((item) => item !== key); | ||
} | ||
triggerActiveKey(keys); | ||
}; |
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.
当前的 onItemClick
函数逻辑虽然是正确的,但代码结构有些迂回,可读性不强。代码首先假定添加 key
,然后根据 accordion
模式和 isActive
状态覆盖这个值。为了提高代码的可读性和可维护性,建议重构此函数,使用更明确的 if/else
结构来处理不同情况。
const onItemClick = (key: React.Key) => {
let newKeys: React.Key[];
if (accordion) {
newKeys = activeKey[0] === key ? [] : [key];
} else {
const index = activeKey.indexOf(key);
const isActive = index > -1;
if (isActive) {
newKeys = activeKey.filter((item) => item !== key);
} else {
newKeys = [...activeKey, key];
}
}
triggerActiveKey(newKeys);
};
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: 1
🧹 Nitpick comments (2)
src/Collapse.tsx (2)
10-18
: 建议优化类型安全性
getActiveKeysArray
函数实现正确,但可以通过更严格的类型检查来提高代码质量。function getActiveKeysArray(activeKey: React.Key | React.Key[]): React.Key[] { let currentActiveKey = activeKey; if (!Array.isArray(currentActiveKey)) { - const activeKeyType = typeof currentActiveKey; - currentActiveKey = - activeKeyType === 'number' || activeKeyType === 'string' ? [currentActiveKey] : []; + currentActiveKey = + currentActiveKey !== undefined && currentActiveKey !== null ? [currentActiveKey] : []; } return currentActiveKey.map((key) => String(key)); }这种方式可以更好地处理
null
和undefined
值,同时简化逻辑。
54-65
: 建议简化 onItemClick 逻辑当前实现功能正确,但可以通过提前返回来简化逻辑流程。
const onItemClick = (key: React.Key) => { - let keys = [...activeKey, key]; if (accordion) { - keys = activeKey[0] === key ? [] : [key]; + const keys = activeKey[0] === key ? [] : [key]; + triggerActiveKey(keys); + return; } + const index = activeKey.indexOf(key); const isActive = index > -1; - if (isActive) { - keys = activeKey.filter((item) => item !== key); - } + const keys = isActive + ? activeKey.filter((item) => item !== key) + : [...activeKey, key]; triggerActiveKey(keys); };这种方式使手风琴模式和多选模式的逻辑更加清晰分离。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json
(1 hunks)src/Collapse.tsx
(2 hunks)
🔇 Additional comments (3)
package.json (1)
50-50
: 依赖版本升级看起来不错!将
@rc-component/util
从^1.0.1
升级到^1.3.0
以支持新的useControlledState
和useEvent
hooks 是合理的。这个版本升级幅度适中,应该不会引入破坏性变更。src/Collapse.tsx (2)
2-2
: 成功迁移到新的状态管理模式从
useMergedState
迁移到useControlledState
和useEvent
的组合是一个很好的重构,使代码更加清晰和符合现代 React 模式。
41-45
: 状态管理实现正确使用
useControlledState
处理受控/非受控模式,并通过getActiveKeysArray
派生activeKey
的实现很清晰。这种模式能够正确处理各种输入类型。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #400 +/- ##
==========================================
+ Coverage 99.15% 99.20% +0.04%
==========================================
Files 5 5
Lines 118 125 +7
Branches 43 46 +3
==========================================
+ Hits 117 124 +7
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
关联issue:ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit