-
-
Notifications
You must be signed in to change notification settings - Fork 244
refactor: Upgrade utils and replace useMergedState #892
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 #892
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本次变更包含两处:在 Changes
Sequence Diagram(s)sequenceDiagram
actor User as 用户
participant Tabs as Tabs 组件
participant Hook as useControlledState
User->>Tabs: 传入 props: activeKey / defaultActiveKey / id
Tabs->>Hook: 初始化 Hook(defaultActiveKey ?? tabs[0]?.key, activeKey)
Hook-->>Tabs: 返回 [value, setValue]
Tabs-->>User: 基于 value 渲染激活的 Tab
Note right of Hook: id 同样通过\nuseControlledState(null, id) 管理
User->>Tabs: 变更 activeKey(受控)或触发内部 setValue
Tabs->>Hook: 同步受控 value 或 更新内部 value
Hook-->>Tabs: 更新后的 value
Tabs-->>User: 重新渲染
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
✨ 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!
此拉取请求旨在通过升级核心工具库并迁移组件内部的状态管理逻辑来优化 Tabs
组件。这一重构提高了代码的一致性,并利用了库的最新功能,从而可能简化未来的维护工作。
Highlights
- 依赖升级: 将
@rc-component/util
依赖包的版本从^1.2.0
升级到^1.3.0
。 - 状态管理钩子替换: 在
src/Tabs.tsx
文件中,将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. ↩
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
依赖,并使用 useControlledState
替换了 useMergedState
。这是一个很好的重构。不过,在 src/Tabs.tsx
中,对 useControlledState
的一处使用可能会引入一个关于 defaultActiveKey
的潜在 bug,当其值为空字符串时,组件的行为会与之前不同。请参考具体的审查意见进行修改。
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/Tabs.tsx (2)
126-129
: 用 Null 合并运算符替代逻辑或,避免把空字符串当作“未提供”
defaultActiveKey || tabs[0]?.key
会在defaultActiveKey === ''
时错误回落到首个 Tab。用??
更符合“未定义/空值才回退”的语义。应用如下 diff:
- const [mergedActiveKey, setMergedActiveKey] = useControlledState<string>( - defaultActiveKey || tabs[0]?.key, - activeKey, - ); + const [mergedActiveKey, setMergedActiveKey] = useControlledState<string>( + defaultActiveKey ?? tabs[0]?.key, + activeKey, + );
145-145
: 显式标注泛型,避免类型推断漂移当前未显式泛型时,T 可能被推断为
string | null
,但可读性一般。建议明确为string | null
,便于后续维护与约束。- const [mergedId, setMergedId] = useControlledState(null, id); + const [mergedId, setMergedId] = useControlledState<string | null>(null, id);另外可选:为保持一致性,外层
<div>
的id
也可考虑使用mergedId
(而不是直接用id
)以在未传入id
时仍获得可预期的可访问性标识。(若担心行为变化,可作为小版本跟进。)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json
(1 hunks)src/Tabs.tsx
(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Tabs.tsx (1)
docs/examples/mix.tsx (1)
activeKey
(20-174)
🔇 Additional comments (1)
package.json (1)
42-42
: 升级 @rc-component/util 至 ^1.3.0:验证 useControlledState 导出及兼容性
- 确认 v1.3.0 确实导出
useControlledState
,且受控判断为value !== undefined
,行为与预期一致- 在代码中替换/移除所有
useMergedState
并使用useControlledState
,可执行:rg -nP -C2 '\b(useMergedState|useControlledState)\b' -g '*.ts' -g '*.tsx'
// Accessibility https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Tab_Role | ||
import classNames from 'classnames'; | ||
import useMergedState from '@rc-component/util/lib/hooks/useMergedState'; | ||
import useControlledState from '@rc-component/util/lib/hooks/useControlledState'; |
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.
💡 Verification agent
🧩 Analysis chain
切换到 useControlledState:导入路径与语义检查
导入路径看起来与项目现有 util 用法一致(同为 /lib/ 前缀)。请确认该导入在 CJS/ESM 打包下都可用,且 useControlledState
的受控判断与 useMergedState
一致,避免受控/非受控行为改变。
🌐 Web query:
@rc-component/util/lib/hooks/useControlledState 是否为稳定入口;CJS/ESM 使用示例;与 useMergedState 行为差异
💡 Result:
简短结论
- @rc-component/util/lib/hooks/useControlledState 不是对外稳定的「官方入口」。库里公开的 hook 名称是 useMergedState(源码/类型里可见),并且包的 CJS/ESM 导出策略历史上有不稳定/兼容问题,建议按包的对外导出方式来引用(不要硬拼 /lib 未声明的子路径)。[1][2][3]
为什么这么说(要点)
- 源码/类型:仓库中 hook 源码文件名为 useMergedState.ts,但其 d.ts/实现里存在以 useControlledState 做默认导出/类型别名的情况 —— 说明内部实现/命名和对外暴露并不完全一致,容易被视为内部实现细节而非稳定 API。使用应以仓库/文档的对外名称为准。
- 包导出策略:rc-util 的 package.json 将主入口指向 ./lib 和 ./es,并且社区已就 /es / /lib 的子路径导出与 ESM/CJS 兼容性提出 Issue(建议通过 package.json 的 exports 统一声明),说明直接引用 /lib/* 或 /es/* 可能因为 packageExports 策略或版本不同而断裂。
CJS / ESM 使用示例(推荐两种稳妥方式)
- 从包顶层导入(首选,依赖包的 exports/main/module 保证)
- ESM:
import { useMergedState } from 'rc-util'; // 或 import useMergedState from 'rc-util/es/hooks/useMergedState'(见下) - CJS:
const { useMergedState } = require('rc-util');
- 明确从 es/lib 子路径(风险:若包未在 exports 声明,会随版本变化出问题)
- ESM:
import useMergedState from 'rc-util/es/hooks/useMergedState'; - CJS:
const useMergedState = require('rc-util/lib/hooks/useMergedState').default;
注意:避免直接依赖未承诺为 public API 的路径名(例如 /lib/hooks/useControlledState),因为 package 的 exports/esm 改动或重命名会导致断裂。
useControlledState vs useMergedState 行为差异(要点)
- useMergedState(rc-util 中的实现)目的是处理受控/非受控两种模式的合并:支持 defaultValue、value、onChange、postState 等选项,返回 [state, setState],并在内部以传入的 value 优先(受控)否则使用内部 state(非受控)。实现细节还包含对 postState 的转换等。
- “useControlledState” 作为通用命名在不同项目/实现中含义不完全一致:有实现会额外返回 isControlled、initialValue 或在受控/非受控切换时发出警告(见社区/项目的不同实现)。因此不要把某个仓库的 useControlledState 当成通用契约;在 rc-util 的上下文,关注 useMergedState 的行为和签名更可靠。
快速参考(阅读/引用)
- useMergedState 源码(react-component/util 仓库)。
- rc-util package.json(显示 main/module/files,说明有 lib/es 两套输出)。
- 社区关于修复 ESM/exports 的 Issue(说明直接从 /lib 或 /es 子路径导入的兼容风险)。
需要我做的下一步?
- 我可以为你检查当前你项目所安装的 rc-util 版本(package.json / node_modules)后,给出针对该版本的确切可用导入路径和示例。要我查就把版本或项目 package.json 发来。
使用官方导出 useMergedState 替换私有路径引用
直接引用 '@rc-component/util/lib/hooks/useControlledState
' 属于未承诺的内部子路径,可能因 CJS/ESM 打包或后续版本变更而失效。请改为:
import { useMergedState } from '@rc-component/util';
(若必须使用子路径,可在 ESM 环境下谨慎使用:
import useMergedState from '@rc-component/util/es/hooks/useMergedState';
但仍存在兼容风险)
确保返回值和受控/非受控逻辑(支持 defaultValue、value、onChange、postState,以 value 优先)与原实现保持一致。
🤖 Prompt for AI Agents
In src/Tabs.tsx around line 3, replace the private subpath import of
useControlledState with the official export from '@rc-component/util' (import {
useMergedState } from '@rc-component/util') to avoid fragile internal path
usage; update any local references to useControlledState to useMergedState and
ensure the hook is used so its return shape and controlled/uncontrolled
semantics remain identical to the previous behavior: support defaultValue,
value, onChange and postState, and prefer value over defaultValue when present
so controlled mode wins.
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 #892 +/- ##
=======================================
Coverage 98.98% 98.98%
=======================================
Files 18 18
Lines 786 787 +1
Branches 236 230 -6
=======================================
+ Hits 778 779 +1
Misses 8 8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
关联issue:ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit
重构
杂务