refactor: drop rc-virtual-list and classnames #351
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough本次提交统一迁移依赖与导入：将 classnames 替换为 clsx，rc-util 与 rc-resize-observer 的内部路径迁移到 Changes
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 (beta)
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 @li-jia-nan, 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 focuses on modernizing the project's dependency stack by migrating from 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
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #351 +/- ##
==========================================
- Coverage 97.86% 97.48% -0.38%
==========================================
Files 19 19
Lines 796 796
Branches 189 193 +4
==========================================
- Hits 779 776 -3
- Misses 17 20 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on updating dependencies, most notably replacing the classnames library with clsx and migrating from rc-util to the scoped package @rc-component/util. These changes are applied consistently across the codebase, including source files, examples, and tests. The update to clsx is a good improvement for performance and bundle size. Additionally, the PR includes minor stylistic enhancements and code cleanup, such as standardizing arrow function syntax and removing trailing whitespace. The changes are well-implemented and I have no further recommendations.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.gitignore(1 hunks)examples/animate.tsx(5 hunks)package.json(2 hunks)src/Filler.tsx(2 hunks)src/List.tsx(2 hunks)src/ScrollBar.tsx(5 hunks)src/hooks/useFrameWheel.ts(1 hunks)src/hooks/useMobileTouchMove.ts(1 hunks)src/hooks/useScrollDrag.ts(1 hunks)src/hooks/useScrollTo.tsx(1 hunks)tests/scroll.test.js(3 hunks)tests/scrollWidth.test.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 (13)
tests/scrollWidth.test.tsx (1)
4-4: 测试 domHook 迁移到@rc-component/util的兼容性需要确认这里将
spyElementPrototypes的来源从rc-util/lib/test/domHook切到了@rc-component/util/lib/test/domHook，逻辑本身不变，但需要确保：
- 新包中导出的
spyElementPrototypesAPI 与旧版完全一致（签名、行为、mock 恢复方式相同）。package.json和 Jest 的moduleNameMapper/ 手动 mock 目录已经同步更新到@rc-component/util，否则单测在 CI 环境可能会找不到模块或拿不到 mock。整体修改看起来是机械替换，没有功能层面的风险，只要上面两点在实际环境中验证通过即可。
src/hooks/useFrameWheel.ts (1)
1-1:raf源切换后请确认 API 完全兼容
useFrameWheel中使用了：
raf.cancel(id)raf(() => { ... })raf(() => { ... }, 2)（带第二个参数）从
rc-util/lib/raf切到@rc-component/util/lib/raf后，请确认新实现对上述用法完全兼容（尤其是多帧延迟和cancel的语义），并在本仓库跑一遍单测 / typecheck，避免运行时动画节流行为出现微妙差异。src/hooks/useMobileTouchMove.ts (1)
1-1:useLayoutEffect迁移到@rc-component/util后需验证 SSR 行为这里从
rc-util/lib/hooks/useLayoutEffect切到@rc-component/util/lib/hooks/useLayoutEffect，只要新实现仍旧是在 SSR 环境降级为useEffect、在浏览器使用useLayoutEffect，行为就与之前一致。建议：
- 确认
@rc-component/util的这个 hook 实现与原来保持兼容；- 在 SSR 场景下跑一次渲染（或现有 e2e/单测）确保没有再出现
useLayoutEffect警告。src/hooks/useScrollDrag.ts (1)
1-1:useScrollDrag使用的raf也依赖完整的cancel能力本文件同样将
raf从rc-util/lib/raf切到@rc-component/util/lib/raf，并在拖拽平滑滚动逻辑中依赖：
raf(() => { ... })循环驱动滚动；raf.cancel(id)及时终止滚动。为了避免拖拽滚动残留、无法停止等问题，请在升级后确认：
@rc-component/util/lib/raf的默认导出与旧实现签名一致；- 滚动相关单测（特别是 “mouse down drag” 那一组）在本地和 CI 均通过。
tests/scroll.test.js (3)
5-5:resetWarned迁移到@rc-component/util时注意全局 warning 状态一致性这里将
resetWarned的来源从rc-util/lib/warning切到@rc-component/util/lib/warning，scroll测试依赖它来清空全局告警缓存，验证 “exceed should not warning”。请确认：
- 新包中的
resetWarned行为与旧版完全一致（清空的是同一个 warning 池，而不是互不相干的实例）。- 其他生产代码中使用的
warning也已经统一切换到@rc-component/util/lib/warning，否则会出现测试清的是 A 实例、运行时写的是 B 实例的问题。如果已经在整个仓库统一迁移到
@rc-component/util，当前改动是合理的。
770-770: 仅为空行调整，无功能影响该处只是插入/调整空行，对测试逻辑和行为没有影响，可以保持当前改动或按团队格式化规则处理。
808-808: 同上：格式化空行调整这里同样只是空行变化，不影响逻辑与断言，可忽略。
.gitignore (1)
10-12: 忽略 pnpm lockfile，并保留新 util 的 mocks 目录是合理的
- 增加
pnpm-lock.yaml忽略，符合多包管理工具共存的惯例，不影响 npm/yarn 用户。- 通过
!tests/__mocks__/@rc-component/util/lib把新 util 的 mock 目录从忽略列表中排除，保证 Jest 手动 mock 能被提交和使用，与现有的rc-utilmocks 规则保持一致。这部分配置与本次依赖迁移方向一致，没有额外风险。
src/Filler.tsx (1)
3-3: I'll verify the concerns raised in the review comment about theclsximport statement. Let me start by examining the codebase.
<function_calls>
#!/bin/bashFirst, let's check the current state of src/Filler.tsx
if [ -f "src/Filler.tsx" ]; then
echo "=== src/Filler.tsx (first 100 lines) ===" && head -100 src/Filler.tsx
else
echo "File not found, searching for Filler.tsx"
fd "Filler.tsx" --type f
fi
</function_calls>I need to explore the repository structure and verify the clsx import issue. Let me search for the relevant files:
<function_calls>
#!/bin/bashSearch for Filler.tsx and package.json
echo "=== Finding Filler.tsx ==="
fd "Filler.tsx" --type f 2>/dev/null || find . -name "Filler.tsx" 2>/dev/null | head -5echo ""
echo "=== Finding package.json ==="
fd "package.json" --type f 2>/dev/null | head -5
</function_calls>Let me check what files exist in the repository:
<function_calls>
#!/bin/bashList repository structure
echo "=== Repository root files ==="
ls -la | head -20echo ""
echo "=== Repository structure (first level) ==="
git ls-files | head -30
</function_calls>Let me search for all TypeScript/TSX files and the clsx package information:
<function_calls>
#!/bin/bashSearch for any Filler files and package info
echo "=== Searching for Filler-related files ==="
find . -name "Filler" -o -name "package.json" 2>/dev/null | grep -v node_modules | head -20echo ""
echo "=== Searching for clsx imports ==="
rg "import.*clsx" --type ts --type tsx -n 2>/dev/null | head -20
</function_calls>Let me use web_search to verify the clsx library's export pattern, as this is a critical factual question about how a popular library works:
<function_calls>
clsx library javascript default export named export how to import
</function_calls>I'll also search for the clsx package documentation:
<function_calls>
clsx npm package export pattern import syntax
</function_calls>src/hooks/useScrollTo.tsx (1)
3-7: import 迁移到新工具包，逻辑保持不变这里只是将
raf、useLayoutEffect和warning的来源切换到新的工具包，实现和调用方式与之前一致，看起来不会引入行为变化。建议在本地或 CI 跑一遍现有测试，确认新包的实现与旧版rc-util完全兼容。package.json (1)
49-49: 新增依赖与源码改动一致，建议确认工具链与版本兼容
- 新增的
@types/node、@rc-component/util、clsx、rc-resize-observer与源码中的 import 使用是一一对应的，整体依赖面收紧到了新工具包和 clsx，看起来合理。- 由于这些依赖（特别是
@types/node和@rc-component/util）版本都比较新，建议在目标 Node 版本下重新安装依赖并跑一轮lint/测试，确认编译、类型检查和运行时都没有异常。Also applies to: 70-72
src/List.tsx (1)
1-5: clsx 与 @rc-component/util 替换保持原有行为
clsx通过命名导入使用，并在mergedClassName中组合prefixCls、RTL 修饰类和外部className，语义与原先的classnames实现等价。useEvent与useLayoutEffect从新的工具包导入，仅替换来源，调用时机和依赖项都未改变，对 List 行为不构成影响。整体来看这几处改动是“无逻辑变更”的依赖迁移，代码风格也与 rc 生态的其它仓库保持一致。建议在迁移完成后跑一轮组件测试/示例页面，确保滚动、虚拟化和滚动事件回调在新工具包下工作正常。
Also applies to: 149-149
examples/animate.tsx (1)
6-8: 示例中改用 clsx 与新 useLayoutEffect，不影响原有动效逻辑
MyItem中className={clsx('item', className)}与原先的classnames('item', className)行为一致，依然在基础类名上叠加外部传入的className。useLayoutEffect改为从新工具包导入，仅是来源迁移，卸载时触发onAppear的逻辑不变。onAppearActive参数加括号以及filter/findIndex回调的小改动都只是语法风格调整，不改变筛选、插入逻辑。建议在本地跑一下
examples/animate示例页面，确认列表插入/删除时的过渡动画和数量统计仍然按预期工作。Also applies to: 77-90, 143-143, 158-165
|
标题不对，得改下。 |
li-jia-nan
changed the title
改好了 |
Co-authored-by: afc163 <afc163@gmail.com>
|
README 里也翻一下，顺便把 badges 修复了 |
|
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. |
好了 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.