-
-
Notifications
You must be signed in to change notification settings - Fork 481
refactor: migrate to @rc-component/virtual-list #1177
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough将虚拟列表依赖从 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
Summary of ChangesHello @yoyo837, 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! This pull request performs a migration of the virtual list implementation from the Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
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
This pull request successfully migrates the project from rc-virtual-list to @rc-component/virtual-list. The dependency updates in package.json and import path changes across various TypeScript files (src/BaseSelect/index.tsx, src/OptionList.tsx, tests/Select.test.tsx, tests/__mocks__/rc-virtual-list.tsx) are correctly implemented. However, there is a critical issue in the test files where the CSS selector for the virtual list scrollbar class was updated incorrectly, which will cause tests to fail.
1064b1b to
f3483b4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1177 +/- ##
==========================================
+ Coverage 99.41% 99.47% +0.05%
==========================================
Files 31 31
Lines 1202 1332 +130
Branches 407 494 +87
==========================================
+ Hits 1195 1325 +130
Misses 7 7 ☔ 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)
package.json (1)
54-56: 虚拟列表相关依赖版本请跟进至最新已采纳的建议这里把依赖迁移到
@rc-component/virtual-list并同步升级了rc-overflow、rc-test,方向是对的。上一轮已有 reviewer 建议把@rc-component/virtual-list调整为^1.0.1,建议参考该建议统一版本,并在升级后本地跑一遍全部测试以确认兼容性。Also applies to: 79-79
🧹 Nitpick comments (1)
tests/__mocks__/rc-virtual-list.tsx (1)
1-3: rc-virtual-list 的 Jest mock 现在桥接到新包这个 mock 仍挂在
rc-virtual-list名字下,但内部改为从@rc-component/virtual-list/lib/mock重新导出,可以让遗留的jest.mock('rc-virtual-list')继续工作。建议检查仓库里是否还存在对'rc-virtual-list'的显式 mock/导入;若已全部迁移到@rc-component/virtual-list,可以考虑删除该文件避免混淆。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/__snapshots__/Select.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Tags.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
package.json(2 hunks)src/BaseSelect/index.tsx(1 hunks)src/OptionList.tsx(1 hunks)tests/OptionList.test.tsx(1 hunks)tests/Select.test.tsx(1 hunks)tests/__mocks__/rc-virtual-list.tsx(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). (2)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
src/BaseSelect/index.tsx (1)
4-4: 虚拟列表类型导入路径已对齐新包将
ScrollConfig、ScrollTo的导入切换到@rc-component/virtual-list/lib/List与依赖迁移保持一致,类型使用方式未变,看起来是纯路径调整;请确认该包版本下仍导出同名类型,并跑一遍 TS 编译/测试确保无类型回归。tests/Select.test.tsx (1)
12-12: 测试用的 ScrollConfig 类型源已更新为新虚拟列表包这里的
ScrollConfig类型导入已与源码中使用的@rc-component/virtual-list/lib/List对齐,保持了类型一致性;建议在依赖升级完成后跑一遍测试(含 TS 类型检查),确认该路径在当前版本下可用。src/OptionList.tsx (1)
6-8: OptionList 虚拟列表依赖已成功切换到新命名空间
ListRef、List和ScrollConfig的导入已统一切到@rc-component/virtual-list,与下方listRef.current?.scrollTo和List<...>的使用方式保持一致,属于无行为变化的迁移;请确认新包的默认导出和类型签名与旧版一致,并在本地跑一下与虚拟滚动相关的用例/快照。tests/OptionList.test.tsx (1)
16-19: OptionList 测试的 VirtualList mock 已对齐到新包将
jest.mock目标和jest.requireActual的来源同时切换到@rc-component/virtual-list,保持了原来“包裹真实实现并拦截 scrollTo”的测试行为;请确认 Jest 配置下该模块能正确解析,并跑一遍 OptionList/Select 相关测试,确保不会出现双重 mock 或解析失败的问题。
快照我没动,看上去很久没更新过了,可能没用了
Summary by CodeRabbit
发布说明
✏️ Tip: You can customize this high-level summary in your review settings.