-
Notifications
You must be signed in to change notification settings - Fork 35
refactor: Upgrade utils and replace useMergedState #82
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 #82
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
participant App as 应用 (App)
participant Tour as Tour 组件
participant UCS as useControlledState
participant Steps as steps 列表
App->>Tour: props { current?, defaultCurrent?, open?, defaultOpen?, steps }
Tour->>UCS: mergedCurrent = useControlledState(defaultCurrent || 0, current)
Tour->>UCS: internalOpen = useControlledState(defaultOpen, open)
Tour->>Steps: 读取 steps.length
alt mergedCurrent 越界 (<0 或 ≥ steps.length)
Tour-->>App: mergedOpen = false
else 索引有效
Tour-->>App: mergedOpen = internalOpen ?? true
end
Note right of Tour: 已移除 postState/origin 逻辑;open 由索引边界与 internalOpen 决定
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks (3 passed)✅ Passed checks (3 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!
此拉取请求旨在通过升级核心工具库并采用新的状态管理钩子来优化代码库。这些更改有助于提高代码的现代化程度和可维护性,确保组件状态管理更加规范和高效。
Highlights
- 依赖升级: 将 @rc-component/util 依赖从 ^1.2.1 升级到 ^1.3.0。
- 状态管理钩子替换: 在 src/Tour.tsx 中,将 useMergedState 替换为 useControlledState 来管理 mergedCurrent 和 mergedOpen 状态。
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
您好,感谢您对项目的贡献。本次 Pull Request 将 useMergedState
重构为 useControlledState
,并升级了 @rc-component/util
依赖,这是一个很好的改进。整体实现正确,保留了原有的逻辑。
我有一个小建议,可以在后续的提交中考虑:
- 在
src/Tour.tsx
文件中,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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Tour.tsx (1)
120-127
: useTarget 应传入 mergedOpen 而非 props.open(修复可见性与定位不同步)当前将
open
(外部受控值)传入useTarget
。当组件处于非受控模式或通过handleClose
内部关闭时(internalOpen
改变),useTarget
不会随mergedOpen
变化而触发定位/滚动,可能导致定位滞后或未滚动到视区。建议改为传入mergedOpen
。- const [posInfo, targetElement] = useTarget( - target, - open, + const [posInfo, targetElement] = useTarget( + target, + mergedOpen, gap, mergedScrollIntoViewOptions, inlineMode, placeholderRef, );
🧹 Nitpick comments (1)
src/Tour.tsx (1)
8-9
: 移除未使用的 useMergedState 导入
useMergedState
已不再使用,建议删除以保持代码整洁,并避免 ESLint 报告未使用导入。-import useMergedState from '@rc-component/util/lib/hooks/useMergedState'; import useControlledState from '@rc-component/util/lib/hooks/useControlledState';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json
(1 hunks)src/Tour.tsx
(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: Socket Security: Pull Request Alerts
🔇 Additional comments (3)
src/Tour.tsx (3)
9-9
: 引入 useControlledState 用于受控/非受控合并 — LGTM
替换思路合理,便于拆分 current/open 的控制通道。
67-67
: current 受控化初始化 OK,但请确认重置行为是否符合预期
defaultCurrent || 0
作为初始值合理,但在首次从关闭到打开时会在 Line 83 处将索引强制重置为 0。若current
为受控,这里可能无效;若为非受控,会改变用户关闭前的停留步。请确认这与既有行为一致。
70-74
: mergedOpen 计算逻辑基本正确且与文档一致
- 越界即关闭的边界检查合理。
- 当
internalOpen
为undefined
时回退到true
与当前文档“open 默认值为 true”的描述一致。(npmjs.com)
"@rc-component/portal": "^2.0.0", | ||
"@rc-component/trigger": "^3.0.0", | ||
"@rc-component/util": "^1.2.1", | ||
"@rc-component/util": "^1.3.0", |
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
请确认 1.3.0 是否导出 useControlledState 路径
此处升级为使用 @rc-component/util@^1.3.0
以引入 useControlledState
。为避免发布后 CI 失败,请核对是否确实存在导出路径 @rc-component/util/lib/hooks/useControlledState
,并与本仓库的 TS 配置兼容。
🌐 Web query:
Does @rc-component/[email protected] export a module at '@rc-component/util/lib/hooks/useControlledState'? Provide a link to the file on unpkg/jsDelivr/GitHub.
💡 Result:
Short answer: No — @rc-component/[email protected] does not export a module at "@rc-component/util/lib/hooks/useControlledState". The package (repo: react-component/util) exposes hooks such as useMergedState (e.g. rc-util/lib/hooks/useMergedState) rather than a useControlledState file. See the package page and repository below for the files/tree.
Sources:
[1] jsDelivr package page for @rc-component/util (shows v1.3.0 exists). [2] GitHub repository file tree for react-component/util. [3] unpkg listing showing hooks include useMergedState (not useControlledState).
修复无效的 useControlledState 导入路径
在 @rc-component/[email protected] 中并不存在 lib/hooks/useControlledState
模块,使用该路径会导致编译或运行时错误。可选方案:
- 将相关导入改为
@rc-component/util/lib/hooks/useMergedState
- 或升级到包含
useControlledState
导出的更高版本
请根据项目需求修正 package.json 依赖版本或调整代码中的导入路径。
🤖 Prompt for AI Agents
In package.json around line 45, the dependency "@rc-component/util": "^1.3.0" is
pointing to a version that does not export lib/hooks/useControlledState which
causes compile/runtime failures; fix by either updating this dependency to a
later version that re-exports useControlledState (bump the version in
package.json and run install) or, if you prefer not to upgrade, change all code
imports that reference "@rc-component/util/lib/hooks/useControlledState" to use
"@rc-component/util/lib/hooks/useMergedState" (or the public exported API) and
run a full rebuild to verify no other missing imports remain.
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 (3)
tsconfig.json (3)
15-15
: "types" 仅包含 minimatch 可能屏蔽其他 @types 的自动注入设置 "types" 会限制只加载列出的类型包。若后续需要 node、jest/vitest 或 dumi 的类型,可能被无意排除。除非确有需要,建议移除此项;或一次性列全所需类型(如 "node"、测试框架)。
请确认是否真的需要全局注入 minimatch 类型;若只是模块导入,一般无需在 "types" 声明。
两种修正择一:
- "types": ["minimatch"] + // 移除,回归默认自动发现或
- "types": ["minimatch"] + "types": ["minimatch", "node", "jest"]
17-19
: 收窄 include/exclude,避免编译进入生成物与构建产物当前 include 匹配 "**/*.ts(x)" 会覆盖 .dumi/tmp,且 exclude 仅排除 node_modules。建议显式排除 .dumi 与构建输出目录(如 lib、es、dist、coverage),并把 include 聚焦到 src 与必要的配置文件,提升类型检查与构建性能。
应用示例:
- "include": [".dumirc.ts", "**/*.ts", "**/*.tsx"], - "exclude": ["node_modules"] + "include": [".dumirc.ts", "src/**/*.ts", "src/**/*.tsx", "typings.d.ts"], + "exclude": ["node_modules", ".dumi/**", "lib/**", "es/**", "dist/**", "coverage/**"]
11-14
: 避免把已发布包名作 TS 路径别名 — 删除或改为内部前缀
- 现状:tsconfig.json 的 paths 将 "@rc-component/tour" 指向 "src/Tour.tsx";仓库搜索仅在 README.md(示例,README.md:36)发现 "import Tour from '@rc-component/tour'",源码未发现对该别名的引用。
- 风险:将真实包名映射为 paths 会在发布/消费端造成编译期与运行时解析不一致,可能引发解析冲突。
- 建议:删除该别名或改为内部前缀(例如 "@rc-tour");并在 tsconfig.json 的 exclude/include 中排除 .dumi/tmp 以避免类型检查进入生成物。
可选修正示例:
"paths": { "@/*": ["src/*"], "@@/*": [".dumi/tmp/*"], - "@rc-component/tour": ["src/Tour.tsx"] + "@rc-tour": ["src/Tour.tsx"] },位置:tsconfig.json (paths),README.md:36(示例引用)。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Tour.tsx
(2 hunks)tsconfig.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Tour.tsx
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #82 +/- ##
=======================================
Coverage 99.07% 99.07%
=======================================
Files 10 10
Lines 216 217 +1
Branches 96 98 +2
=======================================
+ Hits 214 215 +1
Misses 2 2 ☔ 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
🧹 Nitpick comments (1)
tsconfig.json (1)
16-18
: include/exclude 一行化,LGTM;建议区分构建与文档的 TS 配置当前
include
仍包含.dumirc.ts
,在 CI 未生成.dumi/tmp
时可能导致类型检查偶发失败。可考虑新增tsconfig.build.json
(仅包含src/**
,排除 dumi 相关)并在打包脚本中改用该配置,以隔离文档/示例与产物构建。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tsconfig.json
(1 hunks)
🔇 Additional comments (1)
tsconfig.json (1)
11-13
: 别名格式调整 OK,但需构建后确认声明文件未泄露源码路径本地运行你给的检查脚本返回 "No files were searched",说明当前仓库没有生成的声明文件(dist/**/*.d.ts),无法验证 d.ts 是否引用了 src/Tour.tsx。
- 操作:先本地执行构建(npm run build / pnpm build / yarn build),然后运行:
rg -n 'src/Tour.tsx' -g 'dist/**/*.d.ts' || echo 'OK: no direct src path in d.ts'- 若构建后 d.ts 中出现 'src/...',在打包阶段加入别名重写(如 tsc-alias 或 rollup-plugin-tsc-alias),或确保声明文件以包名导入。
关联issue:ant-design/ant-design#54854
替换 useMergedState 为 useControlledState
Summary by CodeRabbit
Bug Fixes
Chores