refactor: Upgrade utils and replace useMergedState #940
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在多个 Picker 组件与其钩子中，将状态管理钩子从 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor 用户 as 用户
participant Picker as SinglePicker/RangePicker
participant Panel as PickerPanel
participant UCS as useControlledState
用户->>Picker: 传入 props（mode / picker / value / defaults）
Picker->>UCS: useControlledState(default, controlledValue)
note right of UCS #DFF2E1: 默认值为第一参，受控值为第二参
UCS-->>Picker: 返回 [state, setState]
用户->>Panel: 传入 props（mode / value / pickerValue / defaults）
Panel->>UCS: mergedMode = useControlledState(picker || 'date', mode)
Panel->>UCS: innerValue = useControlledState(defaultValue, value)
Panel->>UCS: mergedPickerValue = useControlledState(defPicker || mergedValue[0] || now, pickerValue)
UCS-->>Panel: 返回各 [state, setState]
Note over Picker,Panel: useMergedState 的对象式 value/postState 逻辑被改为 useControlledState 的直接参数模式
participant DelayHook as useDelayState
DelayHook->>UCS: useControlledState(default, value)
DelayHook->>DelayHook: triggerUpdate -> setState + forceUpdate（强制重渲染）
DelayHook-->>Picker: 更新状态并触发渲染/回调
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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!
此拉取请求通过将各种选择器组件中的状态管理从 useMergedState 迁移到 useControlledState 来进行重构。此更改与 @rc-component/util 库的升级保持一致，确保了受控和非受控组件状态处理的一致性和潜在改进。
Highlights
- 依赖升级: 将
@rc-component/util依赖升级到^1.3.0版本。 - 状态管理钩子替换: 在多个组件中，将
useMergedState替换为useControlledState，以优化状态管理逻辑。
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. ↩
|
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.
Code Review
This pull request successfully upgrades the @rc-component/util dependency and replaces the deprecated useMergedState with useControlledState. The changes are generally correct, but I've found one area for improvement in src/PickerInput/hooks/useRangePickerValue.ts. The refactoring has removed lazy initialization for a state hook, which could lead to a minor performance degradation. My suggestion restores the original lazy initialization behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/PickerPanel/index.tsx (2)
2-2: 从顶层导入 warning：与其它文件导入路径不一致本文件从 '@rc-component/util' 顶层导入 warning，而 RangePicker 等仍使用 '@rc-component/util/lib/warning'。建议统一导入风格，避免包入口差异带来的树摇/类型分辨差异。
- import { useEvent, useControlledState, warning } from '@rc-component/util'; + import { useEvent, useControlledState } from '@rc-component/util'; + import warning from '@rc-component/util/lib/warning';
230-231: 未受控模式对 picker 变更的同步（可选增强）PickerPanel 无“打开态”钩子来复位模式。若外部动态切换 picker（如从 'date' 切到 'month'）且未受控，建议在 picker 变化时同步 mergedMode，避免面板与 picker 不一致。
const [mergedMode, setMergedMode] = useControlledState<PanelMode>(picker || 'date', mode); +// 当外部未受控且 picker 变化时，重置模式 +React.useEffect(() => { + if (mode === undefined) { + setMergedMode(picker || 'date'); + } + // 仅依赖 picker，避免用户在面板内逐级切换时被外部覆盖 +}, [picker]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.json(1 hunks)src/PickerInput/RangePicker.tsx(2 hunks)src/PickerInput/SinglePicker.tsx(2 hunks)src/PickerInput/hooks/useRangePickerValue.ts(2 hunks)src/PickerInput/hooks/useRangeValue.ts(2 hunks)src/PickerPanel/index.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/PickerPanel/index.tsx (1)
src/interface.tsx (1)
PanelMode(86-86)
src/PickerInput/RangePicker.tsx (1)
src/interface.tsx (1)
PanelMode(86-86)
⏰ 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: WIP
🔇 Additional comments (11)
package.json (1)
44-44: 确认 util 升级与 API 匹配
- 验证 @rc-component/[email protected] 顶层导出是否包含 useControlledState 和 warning（可运行 npm view @rc-component/[email protected] exports 或在 node_modules/@rc-component/util 中 grep -R 进行验证）
- 确认仓库内已无 useMergedState 引用：
rg -n "useMergedState"- 安装 father 和 typescript 后在本地执行 npm run compile 与 npm run lint:tsc ，确保编译与类型校验均通过
src/PickerInput/hooks/useRangeValue.ts (2)
1-1: import 迁移到 useControlledState 合理从 @rc-component/util 引入 useControlledState 与 useEvent，一致性良好。
120-121: 用 useControlledState 取代 useMergedState：语义差异提示useControlledState(defaultValue, value) 不再支持 postState/惰性默认值选项；默认值变化后不会自动重置未受控状态（与过去 useMergedState 的行为基本一致）。请确认业务上没有依赖“默认值在 prop 变化时重置内部状态”的语义。
src/PickerInput/SinglePicker.tsx (2)
1-1: 引入 useControlledState 替换 useMergedState顶层从 @rc-component/util 引入，风格与本 PR 其他文件一致。
262-262: 模式状态改用 useControlledState未受控场景下默认以 picker 作为初始值；结合下方 useLayoutEffect 在打开时重置到 picker，可覆盖“默认值不随 prop 动态变化”的潜在差异，符合预期。
src/PickerInput/RangePicker.tsx (2)
1-1: 引入 useControlledState OK与全局迁移方案一致。
313-315: modes 使用 useControlledState默认值设为 [picker, picker]，受控时由 mode 驱动；未受控时结合打开态的重置逻辑可保持历史行为。建议回归测试：切换 activeIndex、切换 picker、打开/关闭时的面板模式是否与预期一致。
src/PickerInput/hooks/useRangePickerValue.ts (2)
1-1: import 调整为 useControlledState一致性良好。
75-83: 默认值计算从惰性变为即时计算：影响极小但请确认此前 useMergedState(() => getDefaultPickerValue(i)) 为惰性初始化；现在直接调用 getDefaultPickerValue(i) 作为初值，仅在首次渲染时使用。该函数开销很小，通常无影响，但请确认 generateConfig.getNow() 不会引入额外副作用。
src/PickerPanel/index.tsx (2)
241-247: 值状态改为 useControlledState与对外 API 兼容；triggerChange 中的变更判定仍以 isSame 比较语义值，行为保持。
280-283: pickerValue 改为 useControlledState默认回退至 defaultPickerValue || mergedValue[0] || now，且在 value[0] 变化时（pickerValue 未受控）用 effect 同步，避免视图日期漂移，逻辑妥当。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #940 +/- ##
=======================================
Coverage 98.80% 98.80%
=======================================
Files 65 65
Lines 2669 2680 +11
Branches 742 719 -23
=======================================
+ Hits 2637 2648 +11
Misses 29 29
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EmilyyyLiu
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/PickerInput/hooks/useDelayState.ts(2 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: WIP
🔇 Additional comments (1)
src/PickerInput/hooks/useDelayState.ts (1)
1-1: 请手动验证 @rc-component/util 导出
当前在沙箱环境中未能找到@rc-component/util模块，可能依赖未安装或路径不符。请在本地安装依赖后，确认useControlledState与useEvent是否在根包（index.d.ts）中导出，且其类型签名（泛型、参数顺序）与预期一致。如发现未导出，请改为从具体路径导入或采用回退方案。
|
@gemini-code-assist，重新 CR。测试用例失败，应该有什么不一致的地方需要看看： |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/PickerInput/hooks/useDelayState.ts (1)
33-37: 修复测试失败：重复值更新时也应触发 onChange失败用例“Picker.Range › fixed open need repeat trigger onOpenChange”期望重复触发
onOpenChange。当前以state !== nextValueRef.current作为触发条件导致第二次相同值更新被抑制，回调仅调用 1 次。建议在此处不做“是否变化”的短路判断，保证重复事件也能通知上层（此前通过增加 forceUpdate 仅保证了重渲染，但未保证回调触发）：
const doUpdate = useEvent(() => { - triggerUpdate(nextValueRef.current); - - if (onChange && state !== nextValueRef.current) { - onChange(nextValueRef.current); - } + const next = nextValueRef.current; + triggerUpdate(next); + // 为满足“fixed open 需要重复触发 onOpenChange”的语义，重复值也回调 + if (onChange) { + onChange(next); + } });请在本改动与上一条“仅在相同值时 forceUpdate”的优化一并落地后复跑测试，预期次数应由 1 次恢复为 2 次。
♻️ Duplicate comments (1)
src/PickerInput/hooks/useDelayState.ts (1)
14-15: 请核对 useControlledState 的参数顺序（很可能应为 value 在前、defaultValue 在后）当前传参为
(defaultValue, value)。多个生态（如 React Spectrum）的 useControlledState 签名是(value, defaultValue, onChange?)，若此处顺序相反，会导致“受控/非受控”语义颠倒，进而影响后续回调触发与渲染节奏。若确认 @rc-component/util 的签名为
(value, defaultValue)，请应用以下修正：- const [state, setState] = useControlledState<T>(defaultValue, value); + const [state, setState] = useControlledState<T>(value, defaultValue);为确保万无一失，请先确认 1.3.0 版本的 d.ts/实现签名：
What is the TypeScript signature of `useControlledState` in `@rc-component/[email protected]`? Is it `(value, defaultValue, onChange?)` or `(defaultValue, value)`?
🧹 Nitpick comments (1)
src/PickerInput/hooks/useDelayState.ts (1)
16-22: forceUpdate 使用建议：仅在“值未变化”时触发以避免双渲染当前
setState(nextState)后无条件forceUpdate({})，会导致状态变化场景产生一次额外渲染。可在“值未变化（Object.is(state, nextState))”时再强制刷新，既保留“相同值也要重渲染”的诉求，又减少不必要渲染。const triggerUpdate = useEvent((nextState: T) => { setState(nextState); - forceUpdate({}); + // 仅当值未变更时强制刷新，避免多一次 render + if (Object.is(nextState, state)) { + forceUpdate({}); + } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/PickerInput/hooks/useDelayState.ts(3 hunks)
🔇 Additional comments (1)
src/PickerInput/hooks/useDelayState.ts (1)
1-1: 引入来源更新 LGTM从 rc-util 切换到 @rc-component/util 并引入 useControlledState 没问题。版本已在 npm 上于 8 天前发布到 1.3.0。(npmjs.com)
关联issue：ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit