-
Notifications
You must be signed in to change notification settings - Fork 59
refactor: Upgrade utils and replace useMergedState #299
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
refactor: Upgrade utils and replace useMergedState #299
Conversation
Warning Rate limit exceeded@zombieJ has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
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将 Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Mentions Component
participant HookOld as useMergedState (旧)
participant HookNew as useControlledState (新)
rect rgb(240,248,255)
Note left of Component: 旧流程
Component->>HookOld: 初始化\n('', { defaultValue, value })
HookOld-->>Component: 返回 [state, setState]
Component->>Component: 读写 state(合并默认/受控)
end
rect rgb(245,255,240)
Note left of Component: 新流程
Component->>HookNew: 初始化\n(defaultValue || '', value)
HookNew-->>Component: 返回 [state, setState]
Component->>Component: 读写 state(受控/非受控,初始值来自 defaultValue)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 分钟 Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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!
This pull request refactors the Mentions
component by updating a core utility dependency and standardizing its state management logic. The change transitions from useMergedState
to useControlledState
, which improves consistency in handling controlled and uncontrolled component states, aligning with modern React patterns and potentially enhancing maintainability.
Highlights
- Dependency Upgrade: The
@rc-component/util
package has been upgraded to version^1.3.0
. - State Management Refactor: The
useMergedState
hook has been replaced withuseControlledState
in theMentions
component for managing its internal value.
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. ↩
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.3.0
,并使用新的 useControlledState
hook 替换了 useMergedState
。这个重构是正确的,并且实现上保留了原有的逻辑。
我在 InternalMentions
组件中发现了一个先前就存在的潜在 bug,它会导致受控模式下的 Mentions
组件值更新不生效。我已经在相关的代码行上留下了具体的反馈和建议。除此之外,其他的修改看起来都很好。
const [mergedValue, setMergedValue] = useControlledState( | ||
defaultValue || '', | ||
value, | ||
); |
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.
将 useMergedState
替换为 useControlledState
的重构是正确的。
然而,这里有一个潜在的先前就存在的问题。当 InternalMentions
在 Mentions
组件内部使用时,此处的 value
prop 始终是 undefined
。这意味着 InternalMentions
实际上总是在非受控模式下运行,其内部的 mergedValue
状态与 Mentions
组件的 mergedValue
状态是分离的。
这会导致一个 bug:当 Mentions
组件作为受控组件并且其 value
prop 从外部更新时,TextArea
中显示的值不会更新。
为了修复这个问题,Mentions
组件需要将其 mergedValue
传递给 InternalMentions
的 value
prop。虽然修复超出了本次变更的范围,但由于你正在处理状态管理逻辑,这可能是一个值得考虑解决的问题。
一个可能的修复方案是在 Mentions
组件中:
<InternalMentions
value={mergedValue}
{...rest}
/>
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
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 (2)
src/Mentions.tsx (2)
174-177
: useMergedState → useControlledState 语义等价,建议用 nullish 合并符新旧实现对受控/非受控合并的核心行为保持一致;旧的 useMergedState 在 value→undefined 时同样会将内部值同步为 undefined,本次替换不引入行为差异。(app.unpkg.com)
为可读性可改为使用 ??(避免与其他类型时的“假值”误判):
-const [mergedValue, setMergedValue] = useControlledState( - defaultValue || '', - value, -); +const [mergedValue, setMergedValue] = useControlledState( + defaultValue ?? '', + value, +);
567-570
: 外层组件同理,保持一致写法与内部一致,推荐用 ?? 提升可读性;功能不变。(app.unpkg.com)
-const [mergedValue, setMergedValue] = useControlledState( - defaultValue || '', - customValue, -); +const [mergedValue, setMergedValue] = useControlledState( + defaultValue ?? '', + customValue, +);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json
(1 hunks)src/Mentions.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (2)
package.json (1)
52-52
: 依赖升级与代码改动匹配,注意清理残留引用已将 @rc-component/util 升级到 ^1.3.0(新增 lib/hooks/useControlledState),与你在 Mentions.tsx 的替换相匹配。建议确认仓库内不再有 useMergedState 的残留导入。自动检查未成功(ripgrep 因仅使用负向 glob 导致未搜索任何文件)。在本地运行下列修正脚本并把输出贴回以完成验证:
#!/bin/bash set -euo pipefail # 搜索残留的 useMergedState / 旧路径导入,排除构建产物与 node_modules rg -n -uu -S -C2 'useMergedState|hooks/useMergedState' --glob '**/*' --glob '!dist/**' --glob '!lib/**' --glob '!node_modules/**' || true rg -n -uu -S -C2 '@rc-component/util' --glob '**/*' --glob '!dist/**' --glob '!lib/**' --glob '!node_modules/**' || truesrc/Mentions.tsx (1)
8-8
: 导入路径正确,签名匹配当前用法useControlledState 在 @rc-component/util@1.3.0 提供且签名为 useControlledState(defaultStateValue, value?),内部已移除 onChange/postState 逻辑,与你的调用方式一致。(app.unpkg.com)
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 (2)
.github/workflows/main.yml (2)
6-6
: 评估是否需要继承全部机密如该测试不依赖任何机密,建议去掉或仅传递必要机密,降低暴露面;同时兼容来自 fork 的 PR。
- secrets: inherit + # 如无机密需求,移除此行;如需,改为仅传递所需的机密映射 + # secrets: + # NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
4-6
: 可选:添加并发控制与最小权限
- 顶层添加并发组,避免 push 与 PR 双触发导致的重复运行。
- 顶层显式最小权限(contents: read)。
可在工作流顶层追加:
permissions: contents: read concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true请确认 rc-test 的
test-npm.yml
Node 版本矩阵未变更测试覆盖范围(如是否仍覆盖 LTS)。可复用上一条脚本查看node-version
配置。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/main.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/main.yml (1)
5-5
: 将可复用工作流引用从@main
固定为 tag 或 commit SHA位置:.github/workflows/main.yml(第5行)
说明:避免使用@main
带来的供应链风险,应将uses:
指向受信任的 tag 或 commit SHA。建议改动(替换 为实际值):
- uses: react-component/rc-test/.github/workflows/test-npm.yml@main + uses: react-component/rc-test/.github/workflows/test-npm.yml@<commit-sha>验证状态:自动验证失败(gh 返回 404),无法检索到 rc-test 仓库的 SHA;在具有 gh 访问权限的环境中运行 gh 脚本以获取 test-npm.yml 的最新 commit SHA 并替换上述 。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #299 +/- ##
==========================================
+ Coverage 98.04% 98.06% +0.01%
==========================================
Files 8 8
Lines 256 258 +2
Branches 60 61 +1
==========================================
+ Hits 251 253 +2
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
关联issue:ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit