-
-
Notifications
You must be signed in to change notification settings - Fork 622
perf: uninstall classnames, install clsx #1370
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将依赖与代码中的 classNames 用法统一替换为 clsx:更新 package.json 依赖,批量修改 docs 示例与 src 下各组件、hook、工具中的 className 组合函数,未改动控制流或公开 API。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ 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 |
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 a performance optimization by migrating 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
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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 replaces the classnames library with clsx for performance improvements. The changes are applied consistently across the codebase, including updating package.json and all usages in the source files and examples. The replacement is correct and I have no further suggestions. This is a good performance enhancement.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1370 +/- ##
==========================================
- Coverage 96.11% 96.09% -0.02%
==========================================
Files 57 57
Lines 3445 3434 -11
Branches 632 632
==========================================
- Hits 3311 3300 -11
Misses 129 129
Partials 5 5 ☔ 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: 2
🧹 Nitpick comments (1)
src/Table.tsx (1)
782-783: 样式合并顺序变更可能改变滚动样式的优先级当前将
styles?.content放在最后:用户样式可覆盖内部的overflowX/overflowY,这与过去实现(可能内部样式在后)不一定一致。请确认这是否为有意的行为变更;若希望库侧滚动样式始终生效,可将顺序调整为用户样式在前。备选(仅当需保证内部滚动样式优先生效):
- style={{ ...scrollXStyle, ...scrollYStyle, ...styles?.content }} + style={{ ...styles?.content, ...scrollXStyle, ...scrollYStyle }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
docs/examples/animation.tsx(2 hunks)docs/examples/virtual-list-grid.tsx(2 hunks)docs/examples/virtual-list.tsx(2 hunks)package.json(1 hunks)src/Body/BodyRow.tsx(4 hunks)src/Body/index.tsx(2 hunks)src/Cell/index.tsx(2 hunks)src/FixedHolder/index.tsx(2 hunks)src/Header/Header.tsx(3 hunks)src/Table.tsx(4 hunks)src/VirtualTable/BodyLine.tsx(4 hunks)src/VirtualTable/VirtualCell.tsx(2 hunks)src/VirtualTable/index.tsx(2 hunks)src/hooks/useRowInfo.tsx(2 hunks)src/stickyScrollBar.tsx(2 hunks)src/utils/expandUtil.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-08T12:53:09.293Z
Learnt from: bbb169
PR: react-component/table#1202
File: src/Table.tsx:903-904
Timestamp: 2024-11-08T12:53:09.293Z
Learning: 在 `src/Table.tsx` 文件的 React 组件 `Table` 中,即使 `bodyScrollLeft` 频繁更新,也需要在 `TableContextValue` 的 `useMemo` 依赖数组中包含 `bodyScrollLeft` 和 `headerCellRefs`,因为每次滚动时重新计算 `TableContextValue` 是解决该问题所必须的。
Applied to files:
src/stickyScrollBar.tsxsrc/Cell/index.tsxsrc/VirtualTable/index.tsxsrc/Body/index.tsxsrc/VirtualTable/BodyLine.tsxsrc/Body/BodyRow.tsxsrc/Header/Header.tsxsrc/Table.tsx
⏰ 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: test / react component workflow
- GitHub Check: test / react component workflow
🔇 Additional comments (9)
src/hooks/useRowInfo.tsx (1)
119-120: 替换成 clsx 保持原有行为
clsx对空值与字符串的处理与先前的classNames等价,这里继续安全地合并rowProps.className,不会引入新的副作用。整体修改符合预期。src/stickyScrollBar.tsx (1)
199-201: clsx 条件 class 合并正确
clsx在对象语法下只会在激活时输出*-active,完全复现了原有classNames的逻辑,改动安全。src/FixedHolder/index.tsx (1)
178-180: clsx 重构覆盖了原逻辑这里用
clsx组合默认className与可选粘性类,真值判断与之前的classNames一致,行为未变。docs/examples/virtual-list.tsx (1)
26-28: 示例中 clsx 替换无行为差异条件 class 的写法与旧实现等价,示例仍能正确标记最后一列。
docs/examples/animation.tsx (1)
30-31: 动画示例的 clsx 合并正常这里合并节点自身与动画附加 class 的逻辑保持一致,
clsx替换后无功能变化。src/Table.tsx (4)
816-829: clsx 等价替换正确,动态类名映射清晰对象语法和条件类名在 clsx 中与 classnames 行为一致,改动安全。
836-836: Panel 标题处替换为 clsx 无副作用与原行为一致,OK。
843-843: 容器 className 使用 clsx 合理保持了可扩展的外部类名合并。
848-848: Footer Panel 处的 clsx 替换正确不会影响渲染结果。
Summary by CodeRabbit