-
-
Notifications
You must be signed in to change notification settings - Fork 145
refactor: Upgrade utils and replace useMergedState #622
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 #622
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将 Cascader 与 Panel 的内部受控/非受控状态由 useMergedState 切换为 useControlledState,内部原始值改为通过 toRawValues 归一化;搜索内部值改为 internalSearchValue 并不在内部处理里触发 onSearch;同时将 package.json 中 @rc-component/util 升级至 ^1.3.0。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as 用户
participant C as Cascader
participant CS as useControlledState
participant TV as toRawValues
participant P as onSearch (prop)
rect rgb(220,240,255)
U->>C: 输入 / 搜索 / 选择
C->>CS: 读写 internalSearchValue / internalValues
CS-->>C: 返回当前内部值与 setter
C->>TV: toRawValues(internalValues)
TV-->>C: rawValues
end
alt source != "blur"
C->>P: (过去可能)onSearch(...) 被触发(已移除)
else source == "blur"
note right of C: onInternalSearch 仅更新 internalSearchValue,\n不在此处触发 onSearch
end
sequenceDiagram
autonumber
participant Panel as Panel
participant CS as useControlledState
participant TV as toRawValues
participant V as useValues
rect rgb(235,255,235)
Panel->>CS: 管理 internalValues (defaultValue, value)
CS-->>Panel: internalValues 与 setter
Panel->>TV: toRawValues(internalValues)
TV-->>V: rawValues -> useValues 产生渲染数据
V-->>Panel: 渲染数据
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ 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!
此拉取请求旨在通过升级 @rc-component/util 库并将其内部状态管理钩子从 useMergedState 迁移到 useControlledState,来重构 Cascader 和 Panel 组件。这一变更旨在统一组件内部的状态管理逻辑,提高代码的可维护性和一致性。
Highlights
- 依赖升级: 将
@rc-component/util依赖从^1.2.1升级到^1.3.0。 - 状态管理钩子替换: 在
src/Cascader.tsx和src/Panel.tsx文件中,将useMergedState替换为useControlledState。 - 代码逻辑调整: 根据
useControlledState的新用法,移除了postState选项,并对rawValues直接应用了toRawValues函数。
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。整体重构做得很好,逻辑保持了一致性。我发现了一个可以移除的未使用导入,具体请看我的评论。
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)
src/Cascader.tsx (3)
9-13: 统一 util 引入并移除未使用的 useMergedState当前同时存在根包与深层路径的混用,且
useMergedState已不再使用。建议统一到根包导出,保持与src/Panel.tsx一致,避免重复打包。-import useId from '@rc-component/util/lib/hooks/useId'; -import useEvent from '@rc-component/util/lib/hooks/useEvent'; -import useMergedState from '@rc-component/util/lib/hooks/useMergedState'; -import useControlledState from '@rc-component/util/lib/hooks/useControlledState'; +import { useEvent, useControlledState, useId } from '@rc-component/util';
296-299: 避免每次渲染重复创建 rawValues 数组,建议 useMemo 缓存
toRawValues(rawValues)对单值场景会返回新数组,作为useValues依赖将导致不必要的重算。用useMemo稳定引用可优化性能。const getMissingValues = useMissingValues(mergedOptions, mergedFieldNames); + // 归一化的 rawValues,避免每次渲染新建引用 + const mergedRawValues = React.useMemo( + () => toRawValues(rawValues), + [rawValues], + ); + // Fill `rawValues` with checked conduction values const [checkedValues, halfCheckedValues, missingCheckedValues] = useValues( multiple, - toRawValues(rawValues), + mergedRawValues, getPathKeyEntities, getValueByKeyPath, getMissingValues, );
360-366: 受控 searchValue 时清空搜索可能无效,建议同时触发 onSearch('')当
showSearch.searchValue受控时,setSearchValue('')可能被外部值覆盖而无效。为保持 autoClear 行为一致,选择后在受控场景也调用一次onSearch('')。const onInternalSelect = useEvent((valuePath: SingleValueType) => { if (!multiple || autoClearSearchValue) { setSearchValue(''); + // 受控 searchValue 时通知外部清空 + if (searchValue !== undefined && onSearch) { + onSearch(''); + } } handleSelection(valuePath); });请补充一个受控
searchValue的用例,验证选择后输入框确实被清空。src/Panel.tsx (1)
101-107: 同样建议缓存归一化的 rawValues,降低 useValues 重算概率与 Cascader 一致,避免对单值场景每次渲染创建新数组导致依赖变更。
// Fill `rawValues` with checked conduction values + const mergedRawValues = React.useMemo( + () => toRawValues(rawValues), + [rawValues], + ); const [checkedValues, halfCheckedValues, missingCheckedValues] = useValues( multiple, - toRawValues(rawValues), + mergedRawValues, getPathKeyEntities, getValueByKeyPath, getMissingValues, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/Cascader.tsx(4 hunks)src/Panel.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/Panel.tsx (2)
src/hooks/useDisplayValues.ts (1)
rawValues(11-62)src/utils/commonUtil.ts (1)
toRawValues(71-81)
src/Cascader.tsx (2)
src/hooks/useDisplayValues.ts (1)
rawValues(11-62)src/utils/commonUtil.ts (1)
toRawValues(71-81)
⏰ 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: test / react component workflow
🔇 Additional comments (5)
package.json (1)
48-48: 统一使用 @rc-component/util 根导出(避免深路径导入)Panel 已使用根包导出(package.json 中 "@rc-component/util": "^1.3.0"),但 Cascader 仍有深路径导入(/lib/...),会导致重复打包与构建差异,需统一改为根包导出。
在仓库根运行下列脚本核对导入来源(上次脚本因 --type tsx 报错 'unrecognized file type: tsx',已修正):#!/bin/bash # 统计 useControlledState 的导入来源(不要使用 --type) rg -nP -C1 "useControlledState" -g '*.ts' -g '*.tsx' | sed -En "s/.*from ['\"](@rc-component\/util[^'\" ]*)['\"].*/\1/p" | sort | uniq -c若发现深路径导入,替换为根导出 "@rc-component/util" 并重新打包验证。
src/Cascader.tsx (1)
254-254: 值状态迁移为 useControlledState:实现意图清晰,LGTM由受控/非受控统一在
useControlledState(defaultValue, value)管理,后续按需再做toRawValues归一化,符合预期。src/Panel.tsx (3)
2-2: 从根包导入 util:与依赖升级一致,LGTM与
package.json升级保持一致,根包导出可减少重复代码。
81-81: 值状态改为 useControlledState:LGTM迁移简洁明了,后续用
toRawValues做消费端归一化,行为与 Cascader 保持一致。
161-162: 尾随逗号与依赖数组对齐:LGTM有助于更清晰的 diff 与减少编辑时的变更噪音。
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #622 +/- ##
=======================================
Coverage 99.83% 99.83%
=======================================
Files 23 23
Lines 612 614 +2
Branches 176 186 +10
=======================================
+ Hits 611 613 +2
Misses 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
♻️ Duplicate comments (1)
src/Cascader.tsx (1)
11-11: 导入更新 LGTM,确认无残留 useMergedState已正确改为
useControlledState。请顺带全仓库清理遗留的useMergedState引用,并确认@rc-component/util版本已升级到满足该 Hook 的版本。可运行下列脚本核验:
#!/bin/bash set -euo pipefail echo "== 查找残留 useMergedState ==" rg -n --hidden -g '!**/node_modules/**' '\buseMergedState\b' || echo "OK: 未发现残留" echo "== 确认 @rc-component/util 版本 ==" if command -v jq >/dev/null 2>&1; then jq -r '.dependencies["@rc-component/util"] // .devDependencies["@rc-component/util"] // "NOT FOUND"' package.json else node -e "const p=require('./package.json');console.log((p.dependencies||{})['@rc-component/util']||(p.devDependencies||{})['@rc-component/util']||'NOT FOUND')" fi
🧹 Nitpick comments (2)
src/Cascader.tsx (2)
253-253: 为 useControlledState 显式标注泛型,避免 TS 推断为 undefined当
defaultValue与value同为undefined时,TS 可能将状态推断为undefined,进而让setRawValues的入参类型发散。显式标注可消除此类抖动。- const [rawValues, setRawValues] = useControlledState(defaultValue, value); + const [rawValues, setRawValues] = useControlledState<InternalValueType | undefined>(defaultValue, value);
272-274: 搜索状态改为 useControlledState + 避免 blur 触发透传:方向正确实现与受控用法对齐,且通过
info.source !== 'blur'避免多次触发外部onSearch,逻辑清晰。另外两点小优化/验证:
- 合法但冗余:既然默认态为
'',建议使用空合并运算符以增强健壮性。- const mergedSearchValue = internalSearchValue || ''; + const mergedSearchValue = internalSearchValue ?? '';
- 受控场景校验:选择项后清空搜索时(下方
onInternalSelect中的setSearchValue('')),若外部通过showSearch.searchValue完全受控,内部setSearchValue可能不生效。请确认是否需要在清空时同时调用外部onSearch?.('')以驱动受控值同步。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Cascader.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Cascader.tsx (2)
src/hooks/useDisplayValues.ts (1)
rawValues(11-62)src/utils/commonUtil.ts (1)
toRawValues(71-81)
⏰ 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: test / react component workflow
🔇 Additional comments (1)
src/Cascader.tsx (1)
297-297: 在调用处显式标准化值形态:OK将
rawValues在传入useValues前通过toRawValues归一化,等效替代了旧的postState行为,语义更直观。
src/Cascader.tsx
Outdated
| InternalValueType | undefined, | ||
| SingleValueType[] | ||
| >(defaultValue, { value, postState: toRawValues }); | ||
| const [rawValues, setRawValues] = 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.
看起来和原来的不等价
|
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.
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 (2)
src/Cascader.tsx (2)
341-345: 单选清空时可能向 onChange 传入 undefined,需回退为空路径 []当
nextRawValues为空数组时,nextRawValues[0]为undefined。社区对 Cascader 单选清空的预期通常是[](空路径),否则下游类型和逻辑容易出错。- const triggerValues = multiple ? nextRawValues : nextRawValues[0]; + const triggerValues = multiple ? nextRawValues : (nextRawValues[0] ?? []);
360-366: 受控 searchValue 模式下,autoClearSearchValue 不会清空输入框
setSearchValue('')对受控模式无效,应同时通知外部onSearch('')。否则选中后搜索框不清空,行为与非受控不等价。const onInternalSelect = useEvent((valuePath: SingleValueType) => { if (!multiple || autoClearSearchValue) { - setSearchValue(''); + setSearchValue(''); + // 受控时通知外部同步清空 + if (searchValue !== undefined) { + onSearch?.(''); + } } handleSelection(valuePath); });
🧹 Nitpick comments (3)
src/Cascader.tsx (3)
253-254: 变量名拼写错误且语义不清晰,建议重命名为 internalValue当前
interanlRawValues存在拼写错误;同时该 state 存的是 InternalValueType(未归一的值),命名为 “RawValues” 容易误导。- const [interanlRawValues, setRawValues] = useControlledState(defaultValue, value); - const rawValues = toRawValues(interanlRawValues); + const [internalValue, setInternalValue] = useControlledState(defaultValue, value); + const rawValues = toRawValues(internalValue);此外,请同步将后续对
setRawValues的调用改为setInternalValue(见 Line 331)。
273-275: 使用 nullish 合并更稳妥(语义更明确)
internalSearchValue类型为字符串时用??更贴合“仅处理 null/undefined”的语义。- const mergedSearchValue = internalSearchValue || ''; + const mergedSearchValue = internalSearchValue ?? '';
330-333: 若已按上文重命名 state,请同步更新 setter与 Line 253 的重命名保持一致,避免读者误解为“设置原始值”。
- const triggerChange = useEvent((nextValues: InternalValueType) => { - setRawValues(nextValues); + const triggerChange = useEvent((nextValues: InternalValueType) => { + setInternalValue(nextValues);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Cascader.tsx(3 hunks)src/Panel.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Panel.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/Cascader.tsx (2)
src/hooks/useDisplayValues.ts (1)
rawValues(11-62)src/utils/commonUtil.ts (1)
toRawValues(71-81)
⏰ 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: test / react component workflow
🔇 Additional comments (1)
src/Cascader.tsx (1)
11-11: 切换到 useControlledState 的导入 —— LGTM导入路径与本次重构目标一致。
关联issue:ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit
新功能
Bug Fixes
Chores