-
Notifications
You must be signed in to change notification settings - Fork 10
chore: master merge next #34
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
* feat: only cssvar mode * chore: update * chore: cov
* feat: support disable runtime style * test: add test case * refactor: rename with zeroRuntime
📝 Walkthrough概述重大版本更新至2.0.0-alpha.2,包含重构样式工具生成器以简化返回类型和控制流、移除CSS变量注册器逻辑、添加运行时禁用选项(zeroRuntime)以及更新关键依赖版本。 更改
序列图sequenceDiagram
participant App as 应用组件
participant useToken as useToken钩子
participant genStyleUtils as genStyleUtils
participant StyleRegister as useStyleRegister
rect rgb(240, 248, 255)
note over App,StyleRegister: 旧流程(v1.x)
App->>genStyleUtils: 调用样式生成器
genStyleUtils->>useToken: 获取令牌
useToken-->>genStyleUtils: 返回令牌对象
genStyleUtils->>StyleRegister: 创建CSSVarRegister包装
StyleRegister->>StyleRegister: 包装CSS变量逻辑
StyleRegister-->>genStyleUtils: 返回[wrapCSSVar, hashId, cssVarCls]
genStyleUtils-->>App: 返回3元组结果
end
rect rgb(240, 255, 240)
note over App,StyleRegister: 新流程(v2.0)with zeroRuntime
App->>genStyleUtils: 调用样式生成器
genStyleUtils->>useToken: 获取令牌(含zeroRuntime)
useToken-->>genStyleUtils: 返回令牌+zeroRuntime标志
alt zeroRuntime为真
genStyleUtils->>genStyleUtils: 通过useMemo跳过运行时计算
else zeroRuntime为假
genStyleUtils->>StyleRegister: 直接调用useStyleRegister
end
StyleRegister-->>genStyleUtils: 返回hashId、cssVarCls
genStyleUtils-->>App: 返回2元组结果
end
预估代码审查工作量🎯 4 (复杂) | ⏱️ ~45 分钟
诗
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 @MadCcc, 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 significant upgrade to 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 introduces significant refactoring to the style generation utilities, primarily by removing the need for a JSX wrapper from style hooks and adding a zeroRuntime flag to conditionally disable runtime style injection. These changes simplify the overall API. However, I've identified two critical issues where missing checks for an optional cssVar object could lead to runtime crashes. I've also provided a couple of suggestions to improve code consistency and maintainability. Please see the detailed comments below.
| unitlessCssVar.add(token2CSSVar(key, cssVar.prefix)); | ||
| unitlessCssVar.add(token2CSSVar(key, getCompVarPrefix(component, cssVar.prefix))); |
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.
Accessing cssVar.prefix directly will cause a runtime crash if cssVar is undefined, as it's an optional property. You should use optional chaining (?.) to safely access prefix. This will prevent a potential TypeError.
| unitlessCssVar.add(token2CSSVar(key, cssVar.prefix)); | |
| unitlessCssVar.add(token2CSSVar(key, getCompVarPrefix(component, cssVar.prefix))); | |
| unitlessCssVar.add(token2CSSVar(key, cssVar?.prefix)); | |
| unitlessCssVar.add(token2CSSVar(key, getCompVarPrefix(component, cssVar?.prefix))); |
| ); | ||
|
|
||
| if (cssVar && defaultComponentToken && typeof defaultComponentToken === 'object') { | ||
| if (defaultComponentToken && typeof defaultComponentToken === 'object') { |
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.
The check for cssVar was removed from this if condition, but cssVar.prefix is used inside the block on line 377. This will lead to a runtime error if cssVar is undefined. The check for cssVar should be restored to prevent this crash.
| if (defaultComponentToken && typeof defaultComponentToken === 'object') { | |
| if (cssVar && defaultComponentToken && typeof defaultComponentToken === 'object') { |
| return (prefixCls: string, rootCls: string = prefixCls): string => { | ||
| const { theme, realToken, hashId, token, cssVar, zeroRuntime } = useToken(); | ||
|
|
||
| // Update of `disabledRuntimeStyle` would cause React hook error, so memoized it and never update. |
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.
The comment refers to disabledRuntimeStyle, but the variable is zeroRuntime. To maintain consistency and improve code clarity, it's best to update the comment to reflect the actual variable name being used.
| // Update of `disabledRuntimeStyle` would cause React hook error, so memoized it and never update. | |
| // Update of `zeroRuntime` would cause React hook error, so memoized it and never update. |
| expect(document.head.innerHTML).toContain('@layer parent,test;'); | ||
| }); | ||
|
|
||
| describe('disabledRuntimeStyle', () => { |
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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #34 +/- ##
==========================================
+ Coverage 84.76% 89.89% +5.13%
==========================================
Files 11 11
Lines 302 297 -5
Branches 80 74 -6
==========================================
+ Hits 256 267 +11
+ Misses 46 30 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/genStyleUtils.test.tsx (1)
5-5: 测试调整匹配 genStyleHooks 新返回值这里几处改动整体看起来是对齐实现的重构:
- import 类型从相对路径改为 alias,有助于避免路径漂移。
getDefaultToken改为简单对象,符合GetDefaultToken = null | Token | (fn)的定义,在这一用例中并不依赖其为函数。- 对 hooks 返回值的断言从长度 3 改为 2,匹配现在的
[hashId, cssVarCls]形态。如果希望进一步增强回归保护,可以考虑:
- 断言
current[0](hashId)为非空字符串;- 或断言
current[1](cssVarCls)为undefined/ 期望的 class 前缀(视实现而定)。Also applies to: 49-52, 60-60
src/util/genStyleUtils.ts (2)
134-204: genStyleHooks / genCSSVarRegister 行为变化与 cssVar 依赖这一段的重构主要有两点:
genStyleHooks返回值简化为(prefixCls, rootCls) => [hashId, cssVarCls],与测试中对长度为 2 的期望一致,公共 API 更直接。genCSSVarRegister通过useToken()直接拿到{ cssVar, realToken },在useCSSVarRegister回调中重映射defaultToken的 key 后返回componentToken,最后整体返回cssVar?.key作为类名。这里有一个需要特别留意的点:
UseTokenReturn中cssVar是可选字段,而genCSSVarRegister中直接访问了cssVar.prefix和cssVar.key!,包括测试里的mockConfig也只是提供了一个空对象{}。- 如果某些使用场景下
useToken返回的cssVar为undefined,或key为空,这里向useCSSVarRegister传入key: undefined可能会导致运行时错误,或生成无效的 CSS 变量注册。建议二选一处理,以降低未来踩坑的概率:
- 在
genCSSVarRegister内部对cssVar做一次显式存在性检查(例如if (!cssVar?.key) return undefined;并直接跳过useCSSVarRegister);或- 在 useToken 的实现与类型层面约束
cssVar始终存在且key非空,把cssVar的可选标记去掉,使这一假设在类型系统中可见。Also applies to: 206-261
299-307: zeroRuntime 分支中的 Hook 调用顺序与 Biome 警告新增的 zeroRuntime 逻辑是:
- 先从
useToken()解构出zeroRuntime;- 用
useMemo(() => zeroRuntime, [])固定首帧的值;- 如果
memoizedZeroRuntime为真,直接return hashId,后面的usePrefix/useCSP/useUniqueMemo/useStyleRegister都不会执行;- 否则走原有样式注册流程(同时把
defaultComponentToken重写为 CSSVar 形式并参与 merge)。这与
tests/genStyleUtils.test.tsx中disabledRuntimeStyle的预期是一致的:在零运行时模式下不会调用usePrefix或进行实际样式注入。代价是:
useMemo的依赖数组为空,意味着后续即便配置层改变了zeroRuntime,当前组件实例也不会更新这一行为;从注释来看这是有意为之,用于避免 hooks 顺序错误。静态分析(Biome)现在对这一块给出「hook 在条件分支/早返回之后调用」的错误,主要是因为:
usePrefix/useCSP/useUniqueMemo/useStyleRegister都位于if (memoizedZeroRuntime) return之后;- 以及
useStyleRegister还处于if (typeof getResetStyles === 'function')这样的条件块中。从逻辑上只要
zeroRuntime在单个组件生命周期内不变化,现在的写法对 React 来说是安全的,但静态工具无法推断这一点。为了同时保留当前行为并让 Biome 安静下来,可以考虑:
- 在这些特定调用前加上注释,例如
// biome-ignore lint/correctness/useHookAtTopLevel: zeroRuntime 为静态配置,hook 调用顺序在组件生命周期内不变
明确表明这里是经过审慎权衡的特殊用法。如果未来希望支持在同一组件实例中动态切换 zeroRuntime,那就需要重构这一段逻辑(例如始终调用这些 hooks,只在
useStyleRegister的回调内部根据 zeroRuntime 决定是否真正注册样式),避免出现真实的 hooks 顺序问题。Also applies to: 314-321, 350-350, 373-373, 393-393, 410-410
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(3 hunks)src/hooks/useToken.ts(1 hunks)src/interface/index.ts(0 hunks)src/util/genStyleUtils.ts(9 hunks)tests/genStyleUtils.test.tsx(4 hunks)
💤 Files with no reviewable changes (1)
- src/interface/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
tests/genStyleUtils.test.tsx (1)
src/util/genStyleUtils.ts (1)
SubStyleComponentProps(79-82)
🪛 Biome (2.1.2)
src/util/genStyleUtils.ts
[error] 308-308: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 309-309: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 314-314: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 350-350: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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: test / react component workflow
🔇 Additional comments (4)
package.json (2)
3-3: 版本号与 @ant-design/cssinjs 依赖同步到 2.0.0-alpha.2版本号和核心依赖一起升级到 2.0.0-alpha.2,看起来是对齐 cssinjs v2 alpha 的预发布节奏。建议再确认一下:
- 下游是否都已按「可能有 breaking change 的 alpha」来消费;
- npm 发版时是否会配合使用 alpha dist-tag 或类似机制,避免误覆盖稳定线。
Also applies to: 60-60
32-32: prepublishOnly 新增 --any-branch 与 --no-publish 的影响现在
prepublishOnly只执行编译与 np 本地流程,不再真正 publish,同时允许在任意分支跑。建议确认:
- CI/发布流水线是否仍然依赖
prepublishOnly触发真正的发布;- 是否有其他脚本负责最终的 push / publish,避免出现「本地版本号已 bump,但包未发布或 tag 不一致」的情况。
src/hooks/useToken.ts (1)
33-33: UseTokenReturn 新增 zeroRuntime 字段的类型定义在公共返回类型中新增
zeroRuntime?: boolean,与genStyleUtils里零运行时的分支逻辑对齐。因为是可选字段,对现有调用是向后兼容的,只要:
- useToken 的实际实现在零运行时场景下确实设置了
zeroRuntime: true;- 非零运行时场景继续返回
undefined/false。建议后续在文档或注释中简单说明 zeroRuntime 的语义,避免调用方误用。
tests/genStyleUtils.test.tsx (1)
143-181: disabledRuntimeStyle 用例很好地锁定 zeroRuntime 行为这组测试通过自定义 config:
- 让
useToken返回zeroRuntime: true;- 注入可观测的
usePrefixmock;- 调用
genComponentStyleHook再在组件中执行useStyle;最后断言:
- 组件正常渲染;
usePrefix没有被调用。这很好地验证了零运行时配置下不会触发前缀计算与样式注册。需要注意的是,这依赖于 zeroRuntime 在单个组件生命周期内保持不变;如果未来考虑支持运行时动态切换 zeroRuntime,可能需要重新审视当前实现与测试假设。
Summary by CodeRabbit
发布说明
新功能
更新