Conversation
…al prefix classes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello @zombieJ, 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! 此拉取请求引入了一个新功能,旨在优化组件库中 CSS 变量的管理。通过允许为组件的变体(如紧凑模式或大尺寸模式)注入共享的 CSS 变量,而无需重复的样式注入,它解耦了样式注入和变量注册过程。这使得组件变体能够轻量级地继承基础组件的样式系统,从而提高了代码的可维护性和复用性。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthrough为样式钩子生成器新增公共选项 Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Gen as genStyleUtils
participant Hook as GeneratedHook
participant CSS as useCSSVar
participant DOM as StyleProvider/DOM
Dev->>Gen: 调用 genStyleHooks(tokens, options{ extraCssVarPrefixCls })
Gen-->>Dev: 返回 GeneratedHook
Dev->>Hook: 在组件中调用 GeneratedHook
Hook->>CSS: 调用 useCSSVar(rootCls)
alt extraCssVarPrefixCls 存在
Hook->>CSS: 对每个额外前缀调用 useCSSVar(e.g. custom-a, custom-b)
end
CSS->>DOM: 在样式区注册 CSS 变量 / 规则
DOM->>Dev: 渲染时样式生效
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 庆祝诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #36 +/- ##
==========================================
+ Coverage 89.89% 90.26% +0.36%
==========================================
Files 11 11
Lines 297 298 +1
Branches 74 71 -3
==========================================
+ Hits 267 269 +2
+ Misses 30 29 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/util/genStyleUtils.ts (1)
213-222:⚠️ Potential issue | 🟠 Major在
forEach循环中调用 React Hook 违反了 Hooks 规则。
useCSSVar内部调用了useToken()和useCSSVarRegister(),这两个都是 React Hook。在forEach循环中调用它们违反了 React 的 Hooks 规则(不要在循环中调用 Hook),会触发react-hooks/rules-of-hooksESLint 规则报错。即使extraCssVarPrefixCls长度固定,循环调用 Hook 仍然是禁止的,因为 React 按调用顺序追踪 Hook 状态,循环模式违反了这一基本要求。建议的解决方案有:
- 如果注册的 prefix 数量确实是固定的,显式编写每个 Hook 调用,避免循环。
- 如果数量动态,将 CSS 变量注册逻辑移入
genCSSVarRegister内部,使其接收 prefix 列表参数,在单次 Hook 调用中完成所有注册。- 或者创建一个封装组件来处理额外的 CSS 变量注册。
原建议中预先生成 Hook 数组并在
forEach循环中调用的方案仍存在同样的问题。
🧹 Nitpick comments (1)
tests/genStyleUtils.test.tsx (1)
69-96: 测试用例逻辑正确,覆盖了新功能的核心场景。测试验证了
extraCssVarPrefixCls: ['custom-a', 'custom-b']导致useCSSVarRegister被调用 3 次(test-prefix、custom-a、custom-b),且每次调用的scope参数正确。有一点需要注意:此测试因为 mock 了
useCSSVarRegister,所以不会触发 React 的 Rules of Hooks 校验。在实际使用中,forEach内调用 hook 可能会引发问题(见genStyleUtils.ts的相关评论)。建议补充一个边界测试:extraCssVarPrefixCls为空数组[]时,useCSSVarRegister只被调用 1 次。
There was a problem hiding this comment.
Code Review
This pull request successfully introduces the extraCssVarPrefixCls option to genStyleUtils, allowing for the injection of CSS variables for additional prefix classes without injecting styles. This feature is well-implemented and addresses the described use case for component variants. The addition of a dedicated test case in tests/genStyleUtils.test.tsx to verify the useCSSVarRegister calls for these extra prefixes is excellent and ensures the new functionality works as expected. The code changes are clean and maintainable.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@tests/extraCssVarPrefixCls.test.tsx`:
- Around line 70-72: The failing assertion expects the literal selector
".test-prefix" but the generated CSS may wrap selectors (e.g.,
:where(.test-prefix)) or omit the leading dot, and mocks may be incomplete so
CSS vars aren't emitted; update the test around totalStyle to assert against the
actual output by either matching without the leading dot or using a regex that
tolerates wrappers (reference totalStyle and the generated selector
"test-prefix"/":where(.test-prefix)"), and also verify the theme/mock used by
useCSSVar emits variables—enhance the mock if necessary so CSS variable
injection runs before asserting.
- Line 68: Remove the debug console.log in the test — delete the line that calls
console.log('Total CSS:', totalStyle) in tests/extraCssVarPrefixCls.test.tsx
(search for the console.log referencing totalStyle) so the test no longer emits
noisy output in CI; if you need retained information for assertions, replace it
with an appropriate expect(...) assertion using the totalStyle variable instead
of logging.
- Around line 43-51: The style generator callbacks passed to genStyleHooks (the
2nd and 3rd arguments) return empty objects causing the style-injection to be
skipped; update those callbacks in tests/extraCssVarPrefixCls.test.tsx to return
at least one minimal, valid CSS rule (e.g., a dummy class or an inherited
property) so the injectStyle flow creates the <style> for the `.test-prefix`
prefix and the extraCssVarPrefixCls behavior is exercised.
40cd99a to
aaf1869
Compare
aaf1869 to
485b231
Compare
- Update @ant-design/cssinjs to 2.1.0 which supports scope array - Pass all scopes to single useCSSVarRegister call instead of looping - Generates comma-separated selectors for better performance Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 60: The dependency entry "@ant-design/cssinjs": "2.1.0" in package.json
is invalid; update the version string to a valid published version such as
"2.0.3" or a semver range like "^2.0.3" to allow patch updates, then run npm
install to verify; ensure you change the value for the exact key
"@ant-design/cssinjs" in package.json and commit the updated package.json (and
package-lock.json/yarn.lock if present).
🧹 Nitpick comments (3)
action.md (1)
1-53: 规划文档,无阻塞性问题。该文件为后续
@ant-design/cssinjs的优化规划,不影响当前运行时行为。内容清晰,建议后续完成优化后删除或移至项目 wiki/issue 中,避免长期留存在代码仓库根目录。src/util/genStyleUtils.ts (2)
242-263:tokenGenerator每次渲染都会创建新的函数引用。
tokenGenerator在 hook 体内定义为普通函数,每次渲染都会生成新引用。如果useCSSVarRegister内部对该回调做了引用比较(如 deps 比较或缓存 key),可能导致不必要的重复注册。建议确认useCSSVarRegister内部是否依赖该回调的引用稳定性;如果是,考虑用useCallback或useMemo包裹。
213-220:options?.extraCssVarPrefixCls未做空数组保护。当
extraCssVarPrefixCls为空数组[]时,条件判断为 truthy,会走数组分支传入[rootCls],虽功能上不影响正确性(scope变成单元素数组而非字符串),但与未传该选项时的行为不一致。建议加上长度检查:建议的修改
- const cssVarCls = useCSSVar( - options?.extraCssVarPrefixCls ? [rootCls, ...options.extraCssVarPrefixCls] : rootCls, - ); + const cssVarCls = useCSSVar( + options?.extraCssVarPrefixCls?.length ? [rootCls, ...options.extraCssVarPrefixCls] : rootCls, + );
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@gemini-code-assist, CR again |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
此特性在 genStyleUtils 中新增 extraCssVarPrefixCls 配置选项,用于为自定义 prefixCls 注入 CSS 变量。
设计背景
在组件库开发中,同一组件常存在多个变体(如 compact、large 等),这些变体需要共享基础组件的 CSS 变量,但不必重复注入样式逻辑。extraCssVarPrefixCls
旨在解耦样式注入与变量注册,为变体组件提供轻量级的 CSS 变量支持。
变更内容
适用场景
Summary by CodeRabbit
发布说明
新功能
改进
测试