-
Notifications
You must be signed in to change notification settings - Fork 63
chore: migrate to @rc-component namespace and update #198
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
chore: migrate to @rc-component namespace and update #198
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
autonumber
participant U as 用户
participant R as Rate 组件
participant H as useControlledState (新)
participant S as Star 子项
rect rgb(240,248,255)
note right of R: 主要变更:导入路径与受控状态钩子替换
end
U->>R: 传入 props (value/defaultValue/onChange)
R->>H: 初始化 value(useControlledState(defaultValue || 0, propValue))
H-->>R: 返回 [value, setValue]
U->>R: 鼠标/键盘 交互
R->>S: 传递当前 value 与回调
S-->>R: 触发 change(点击/键盘)
R->>H: setValue(newValue)
H-->>R: 更新 value
R-->>U: 调用 onChange(newValue)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ 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)
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.
Please see the documentation for more information. 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. 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 生态系统对齐。主要目标是标准化命名空间、更新核心依赖以及改进状态管理机制,从而提高代码库的一致性、可维护性和未来兼容性。
Highlights
- 命名空间迁移: 将项目迁移到 @rc-component 命名空间下。包名变更为 @rc-component/rate,但文档和配置中的引用别名更新为 @rc-component/slider。
- 依赖更新: 将 rc-util 依赖更新为 @rc-component/util。
- 状态管理钩子替换: 将 useMergedState 替换为 useControlledState,以优化受控组件的状态管理。
- 构建工具更新: 更新了 father-plugin 依赖并引入了 @rc-component/np。
- 文档和配置更新: 相应地更新了 README.md、示例文件、tsconfig.json 和 .dumirc.ts 中的引用路径和别名。
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 命名空间并更新了相关依赖,这很棒。其中 rc-util 已被正确替换为 @rc-component/util,useMergedState 也已更新为 useControlledState。但是,我发现一个反复出现的问题:在多个文件中,包名被错误地更改为 @rc-component/slider,而正确的应该是 @rc-component/rate。这似乎是一个复制粘贴错误,会导致文档示例和本地开发环境中断。请修正这些包名。
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
59-65: 示例导入路径错误:应为 @rc-component/rateREADME 代码片段导入了
@rc-component/slider,与包名不符,发布后会误导用户。请改为@rc-component/rate。此外,README 顶部标题、安装徽章与 npm 链接仍指向rc-rate,建议一并更新(标题、npm badge、bundlephobia 链接、下载量 badge 等)。-import Rate from '@rc-component/slider'; +import Rate from '@rc-component/rate';-import Rate from '@rc-component/slider'; +import Rate from '@rc-component/rate';Also applies to: 71-84
🧹 Nitpick comments (6)
package.json (2)
41-41: 避免对 bun 的隐式依赖:改为直接调用本地 tsc仓库并未声明对 bun 的依赖,使用
bunx会提高贡献门槛。建议改为直接调用本地安装的 TypeScript。- "tsc": "bunx tsc --noEmit", + "tsc": "tsc --noEmit",
5-7: Node 版本声明过旧(可选)
engines.node仍为>=8.x,而本仓库 dev 依赖(如 TypeScript 5、father 4、dumi 2)通常要求更高的 Node 版本。为减少误导,建议提升到至少 LTS(如 >=14 或 >=16)。- "node": ">=8.x" + "node": ">=16.0.0"src/Star.tsx (2)
46-50: 键盘事件建议改用 e.key 以规避 keyCode 废弃(可选)React 与浏览器均已标记 keyCode 废弃。建议使用
e.key === 'Enter',减少魔法数字依赖。- const onInternalKeyDown: React.KeyboardEventHandler<HTMLDivElement> = (e) => { - if (e.keyCode === KeyCode.ENTER) { - onClick(e, index); - } - }; + const onInternalKeyDown: React.KeyboardEventHandler<HTMLDivElement> = (e) => { + if (e.key === 'Enter') { + onClick(e, index); + } + };
81-90: 事件处理与 ARIA 细节微调(可选)
- 在 TSX 中向事件属性传入
null容易触发类型不兼容;更建议用undefined。- 补充
aria-disabled以提升可访问性。- <div - onClick={disabled ? null : onInternalClick} - onKeyDown={disabled ? null : onInternalKeyDown} - onMouseMove={disabled ? null : onInternalHover} + <div + onClick={disabled ? undefined : onInternalClick} + onKeyDown={disabled ? undefined : onInternalKeyDown} + onMouseMove={disabled ? undefined : onInternalHover} role="radio" aria-checked={value > index ? 'true' : 'false'} aria-posinset={index + 1} aria-setsize={count} + aria-disabled={disabled} tabIndex={disabled ? -1 : 0} >tests/simple.spec.js (1)
294-306: 测试描述中的拼写小问题(可选)it 描述里的 “diabled” 应为 “disabled”,建议统一修正,提升可读性。
- it('range picker should accept onMouseEnter and onMouseLeave event when Rate component is diabled', () => { + it('range picker should accept onMouseEnter and onMouseLeave event when Rate component is disabled', () => {- it('range picker should accept onMouseEnter and onMouseLeave event when Rate component is not diabled', () => { + it('range picker should accept onMouseEnter and onMouseLeave event when Rate component is not disabled', () => {src/Rate.tsx (1)
97-98: useControlledState 切换:确认“0”为受控值;建议用 ?? 替代 || 以优化默认值语义
- 请确认 useControlledState 的“受控/非受控”判断基于 propValue !== undefined,而非 truthy 检查;否则当 value=0 时会被误判为非受控,造成状态不同步。
- 建议将 defaultValue || 0 改为 defaultValue ?? 0,避免将 NaN/-0 等非常规数值误回退为 0(边缘场景,但语义更稳)。
建议微调:
- const [value, setValue] = useControlledState(defaultValue || 0, propValue); + const [value, setValue] = useControlledState(defaultValue ?? 0, propValue);另外,cleanedValue 无受控需求,用 useState 更直接(保留现状也可):
- const [cleanedValue, setCleanedValue] = useControlledState<number | null>(null); + const [cleanedValue, setCleanedValue] = React.useState<number | null>(null);可用用例校验 0 受控场景与半星步进:受控 value=0 时,键盘/点击应只通过 onChange 反馈,内部 state 不自增。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.dumirc.ts(1 hunks)README.md(2 hunks)docs/examples/characterRender.tsx(1 hunks)docs/examples/simple.tsx(1 hunks)package.json(2 hunks)src/Rate.tsx(2 hunks)src/Star.tsx(1 hunks)tests/simple.spec.js(1 hunks)tsconfig.json(1 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 (4)
package.json (1)
2-3: 包名与版本调整看起来合理重命名为
@rc-component/rate且以1.0.0起新主版本,符合迁移到新命名空间的语义化要求。src/Star.tsx (1)
2-2: KeyCode 路径迁移 LGTM从
rc-util/lib/KeyCode迁移到@rc-component/util/lib/KeyCode与依赖变更一致。tests/simple.spec.js (1)
3-3: KeyCode 迁移 LGTM测试从
rc-util/lib/KeyCode迁移到@rc-component/util/lib/KeyCode与源码一致,键值常量不变,行为应保持稳定。src/Rate.tsx (1)
1-3: 确认 @rc-component/util 替换:依赖已替换,需验证导出签名
- 已验证:src/Rate.tsx、src/Star.tsx、tests/simple.spec.js 已改为从 @rc-component/util/lib/... 导入;package.json 已包含 @rc-component/util(^1.3.0);未发现 rc-util 遗留导入。
- 待确认:KeyCode 导出的常量名/值(如 LEFT/RIGHT 等)是否与原 rc-util 完全一致;pickAttrs 的第二参数签名({ aria, data, attr })是否未变化。请确认或允许我进一步在仓库中比对 KeyCode.* 与 pickAttrs(...) 的调用细节。
- 建议(可选):中长期将键盘判断改为 event.key === 'ArrowLeft'/'ArrowRight',减少对已废弃 keyCode 的依赖。
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
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. |
e5ed212 to
b2e42f1
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #198 +/- ##
=======================================
Coverage 96.95% 96.95%
=======================================
Files 4 4
Lines 164 164
Branches 54 54
=======================================
Hits 159 159
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
now.json (1)
4-13: now.json 的 dist 配置与 docs 构建产物目录不一致,可能导致部署空目录package.json 中
now-build执行的是npm run docs:build(dumi 默认输出到.doc),而此处distDir设置为dist,路由也指向/dist。这会导致部署目录与构建产物不一致,页面 404。两种修复任选其一:
- 方案 A:保持脚本不变,改 now.json 指向
.doc:- "config": { "distDir": "dist" } + "config": { "distDir": ".doc" } ... - { "src": "/(.*)", "dest": "/dist/$1" } + { "src": "/(.*)", "dest": "/.doc/$1" }
- 方案 B:保持 now.json 不变,修改 package.json 的
now-build,在构建后拷贝到 dist:- "now-build": "npm run docs:build", + "now-build": "npm run docs:build && shx rm -rf dist && shx cp -r .doc dist",(需将
shx加到 devDependencies 或改用等价的 shell 命令)
♻️ Duplicate comments (2)
package.json (2)
2-4: 版本号从 2.13.1 重置到 1.0.0 的语义需确认命名空间迁移通常不要求回到 1.0.0。若 API 有破坏性变化,建议使用 3.0.0;若无破坏性变化,建议沿用 2.x 的语义递增。若团队已有版控策略,此条可忽略。
43-45: 确认 useControlledState 的来源,@rc-component/util 可能未导出该 hook本 PR 目标为以
useControlledState替换useMergedState,但仅新增了@rc-component/util依赖。需要确认该包是否提供该 hook;若没有,构建会在src/Rate.tsx的导入处失败。可选方案:
- 回退为
rc-util的useMergedState;- 在仓内实现一个极简的
useControlledState并从本地导入;- 引入实际提供该 hook 的包。
🧹 Nitpick comments (1)
now.json (1)
3-3: 确认 Vercel/Now 项目名是否支持带作用域(@/)的字符串
"name": "@rc-component/rate"可能在部分部署/仪表盘里被当作不合法项目名(包含@、/)。建议确认实际部署平台的限制;若有问题,改为不含特殊字符的别名(例如rc-component-rate)。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.dumirc.ts(1 hunks)README.md(2 hunks)docs/examples/characterRender.tsx(1 hunks)docs/examples/simple.tsx(1 hunks)now.json(1 hunks)package.json(2 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .dumirc.ts
- README.md
- docs/examples/simple.tsx
- docs/examples/characterRender.tsx
- tsconfig.json
| "main": "./lib/index", | ||
| "module": "./es/index", |
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.
🛠️ Refactor suggestion
缺少 types 字段,TypeScript 消费者可能无法获得类型入口
既然通过 father 输出 es/ 与 lib/,通常会同时生成 .d.ts。建议显式声明:
"main": "./lib/index",
"module": "./es/index",
+ "types": "./es/index.d.ts",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "main": "./lib/index", | |
| "module": "./es/index", | |
| "main": "./lib/index", | |
| "module": "./es/index", | |
| "types": "./es/index.d.ts", |
🤖 Prompt for AI Agents
In package.json around lines 20-21, the package lacks a "types" field so
TypeScript consumers won't know the declaration entry; add a "types" field that
points to the generated .d.ts entry (for example the lib build output like
"./lib/index.d.ts" or wherever father emits declarations) and, if you publish
both ESM and CJS with different .d.ts locations, ensure the field points to the
canonical declaration file (and optionally add a "typesVersions" map if you need
to support alternate paths for different TS versions).
| "compile": "father build && lessc assets/index.less assets/index.css", | ||
| "coverage": "rc-test --coverage", | ||
| "docs:build": "dumi build", | ||
| "docs:deploy": "gh-pages -d .doc", | ||
| "compile": "father build && lessc assets/index.less assets/index.css", | ||
| "prepare": "dumi setup", | ||
| "prepublishOnly": "npm run compile && np --yolo --no-publish", | ||
| "postpublish": "npm run docs:build && npm run docs:deploy", | ||
| "lint": "eslint src/ --ext .ts,.tsx,.jsx,.js,.md", | ||
| "now-build": "npm run docs:build", | ||
| "prepare": "dumi setup", | ||
| "prepublishOnly": "npm run compile && rc-np", | ||
| "prettier": "prettier --write \"**/*.{ts,tsx,js,jsx,json,md}\"", | ||
| "postpublish": "npm run docs:build && npm run docs:deploy", | ||
| "start": "dumi dev", | ||
| "test": "rc-test", | ||
| "coverage": "rc-test --coverage", | ||
| "now-build": "npm run docs:build" | ||
| "tsc": "bunx tsc --noEmit" | ||
| }, |
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.
构建/发布脚本存在两点风险:bunx 依赖外部环境;now-build 与 now.json 不匹配
tsc使用bunx会在未安装 Bun 的环境(多数 CI、开发机)失败。应改为使用本地 devDependency 的tsc。now-build仅执行docs:build,与 now.json 的dist指向不一致,可能部署空目录(详见 now.json 评论)。
建议修改:
- "now-build": "npm run docs:build",
+ "now-build": "npm run docs:build", // 若采用 now.json 方案 A;否则参考 now.json 方案 B
@@
- "tsc": "bunx tsc --noEmit"
+ "tsc": "tsc --noEmit"如果仓库目前未安装 TypeScript 的可执行文件,请确保 typescript 已在 devDependencies(此处已存在 ^5.0.4,可直接使用)。
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In package.json lines 28-41, the tsc script uses bunx which will fail when Bun
is not installed and now-build only runs docs:build which doesn't produce the
dist folder referenced by now.json; change "tsc" to invoke the local TypeScript
compiler (use the package's devDependency binary, e.g., replace bunx tsc
--noEmit with just tsc --noEmit or use npm/yarn exec to call the local tsc) and
update "now-build" to run the script that actually emits the deployment output
(e.g., run the compile script or whichever build produces the dist directory
referenced in now.json) and ensure typescript is present in devDependencies so
the local tsc binary is available.
| "react": "^16.0.0", | ||
| "react-dom": "^16.0.0", | ||
| "typescript": "^5.0.4" | ||
| }, | ||
| "peerDependencies": { | ||
| "react": ">=16.9.0", | ||
| "react-dom": ">=16.9.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.
peerDependencies 与本地 dev React 版本不匹配
peer 要求 react/react-dom >=16.9.0,但 devDependencies 中是 ^16.0.0。这会导致本地开发/测试未覆盖到真实的 peer 下限并可能出现类型/行为差异。
建议至少对齐到 16.14(React 16 的最后一个小版本):
- "react": "^16.0.0",
- "react-dom": "^16.0.0",
+ "react": "^16.14.0",
+ "react-dom": "^16.14.0",或直接提升到更高的 LTS 支持版本(若项目策略允许)。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "react": "^16.0.0", | |
| "react-dom": "^16.0.0", | |
| "typescript": "^5.0.4" | |
| }, | |
| "peerDependencies": { | |
| "react": ">=16.9.0", | |
| "react-dom": ">=16.9.0" | |
| }, | |
| "react": "^16.14.0", | |
| "react-dom": "^16.14.0", | |
| "typescript": "^5.0.4" | |
| }, | |
| "peerDependencies": { | |
| "react": ">=16.9.0", | |
| "react-dom": ">=16.9.0" | |
| }, |
🤖 Prompt for AI Agents
In package.json around lines 66 to 73, the devDependencies list react/react-dom
as ^16.0.0 which conflicts with peerDependencies requiring >=16.9.0; update the
devDependencies to match the peer lower bound (recommend at least 16.14.0 for
React 16 final patch) or bump both devDependencies and peerDependencies to a
supported LTS version per project policy so local dev/test runs use a React
version compatible with the declared peer range.
| "engines": { | ||
| "node": ">=8.x" | ||
| } |
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.
🛠️ Refactor suggestion
engines 声明过低(>=8.x),与现有工具链不兼容并且已 EOL
Node 8 早已 EOL,且与 typescript@^5、father@^4、dumi@^2 等工具不兼容。建议将最低版本提升到维护中的 LTS(建议 ≥14.18 或 ≥16.14,视工具链支持而定)。
推荐:
- "engines": {
- "node": ">=8.x"
- }
+ "engines": {
+ "node": ">=16.14"
+ }并在 CI 中同步调整 Node 版本矩阵。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "engines": { | |
| "node": ">=8.x" | |
| } | |
| "engines": { | |
| "node": ">=16.14" | |
| } |
🤖 Prompt for AI Agents
In package.json around lines 74–76, the "engines" field currently pins Node to
">=8.x" which is EOL and incompatible with our toolchain; update it to a
maintained LTS baseline (recommend ">=16.14.0" or at minimum ">=14.18.0"
depending on downstream support) and ensure the string uses full semver (e.g.
">=16.14.0"); after updating package.json, mirror the same Node versions in CI
(update the Node matrix in workflow files) so tests/builds run against the new
minimum and any additional supported versions.
依赖rc-util修改为 @rc-component/util
替换 useMergedState 为 useControlledState
Summary by CodeRabbit