fix: apply StyleProvider nonce to CSS variable styles#252
Conversation
|
🎊 PR Preview d7dbdb7 has been successfully built and deployed to https://ant-design-cssinjs-preview-pr-252.surge.sh 🕐 Build time: 91.283s
🤖 By surge-preview |
Summary of ChangesHello, 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! 此拉取请求旨在解决 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
|
📝 WalkthroughWalkthrough在 StyleContextProps 中新增可选 变更
序列图sequenceDiagram
participant React as React Component
participant StyleProvider as StyleProvider
participant StyleContext as StyleContext
participant Hook as useCacheToken / useCSSVarRegister
participant merge as mergeCSSConfig
participant update as updateCSS
React->>StyleProvider: Render with nonce prop
StyleProvider->>StyleContext: provide nonce
React->>Hook: invoke hook (needs CSS update)
Hook->>StyleContext: read nonce
Hook->>Hook: compute nonceStr (invoke if function)
Hook->>merge: mergeCSSConfig(config, nonceStr)
merge-->>Hook: mergedCSSConfig
Hook->>update: call updateCSS(..., mergedCSSConfig)
update->>update: create/update <style> with nonce attribute
update-->>React: style tag inserted/updated
预估代码审查工作量🎯 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 #252 +/- ##
==========================================
+ Coverage 94.65% 94.71% +0.06%
==========================================
Files 32 32
Lines 2823 2859 +36
Branches 448 453 +5
==========================================
+ Hits 2672 2708 +36
Misses 151 151 ☔ 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
本次 PR 修复了 StyleProvider 的 nonce 属性未能正确应用于 CSS 变量样式的问题，通过在 StyleContext 中传递 nonce 并在 useCacheToken 和 useCSSVarRegister 中消费它来解决。代码变更逻辑清晰，并且添加了相应的测试用例。
我有两点建议：
- 在
useCacheToken和useCSSVarRegister中存在重复的代码逻辑，建议提取为共享函数以提高可维护性。 - 新增的测试用例只覆盖了
useCacheToken的场景，建议补充对useCSSVarRegister的测试以确保修复的完整性。
src/hooks/useCSSVarRegister.ts
Outdated
| const mergedCSSConfig: Parameters<typeof updateCSS>[2] = { | ||
| mark: ATTR_MARK, | ||
| prepend: 'queue', | ||
| attachTo: container, | ||
| priority: -999, | ||
| }); | ||
| }; | ||
|
|
||
| const nonceStr = typeof nonce === 'function' ? nonce() : nonce; | ||
| if (nonceStr) { | ||
| mergedCSSConfig.csp = { nonce: nonceStr }; | ||
| } | ||
|
|
||
| const style = updateCSS(cssVarsStr, styleId, mergedCSSConfig); |
There was a problem hiding this comment.
这部分用于创建 mergedCSSConfig 并处理 nonce 的逻辑与 src/hooks/useCacheToken.tsx 中的代码几乎完全相同。为了遵循 DRY (Don't Repeat Yourself) 原则并提高代码的可维护性，建议将这部分逻辑提取到一个共享的工具函数中。
例如，可以创建一个工具函数，接收 nonce 和 container 作为参数，并返回 updateCSS 所需的配置对象。
// 在某个 util 文件中
function getUpdateCSSConfig(nonce, container) {
const config = {
mark: ATTR_MARK,
prepend: 'queue',
attachTo: container,
priority: -999,
};
const nonceStr = typeof nonce === 'function' ? nonce() : nonce;
if (nonceStr) {
config.csp = { nonce: nonceStr };
}
return config;
}然后在 useCSSVarRegister 和 useCacheToken 中调用此函数。
| describe('StyleProvider nonce for CSS var', () => { | ||
| function testWithStyleProvider(name: string, nonce: string | (() => string)) { | ||
| it(name, () => { | ||
| const { unmount } = render( | ||
| <StyleProvider cache={createCache()} nonce={nonce}> | ||
| <Box /> | ||
| </StyleProvider>, | ||
| ); | ||
|
|
||
| const styles = Array.from(document.head.querySelectorAll('style')); | ||
| // Box 组件使用 useCacheToken 注册 CSS var，应该有 nonce | ||
| const cssVarStyle = styles.find(s => s.innerHTML.includes('--primary-color')); | ||
| expect(cssVarStyle).toBeDefined(); | ||
| expect(cssVarStyle?.nonce).toBe('bamboo'); | ||
| // unmount 后样式清理行为取决于 cache 配置 | ||
| }); | ||
| } | ||
|
|
||
| testWithStyleProvider('string', 'bamboo'); | ||
| testWithStyleProvider('function', () => 'bamboo'); | ||
| }); |
There was a problem hiding this comment.
这个测试套件很好地覆盖了 StyleProvider 的 nonce 属性在 useCacheToken 中的应用场景。但是，本次修复同样涉及 useCSSVarRegister，目前缺少对这个 hook 的 nonce 功能的测试覆盖。
建议增加一个测试用例来验证 nonce 在 useCSSVarRegister 中也能正常工作，以确保修复的完整性。可以创建一个使用 useCSSVarRegister 的新组件，并在 StyleProvider 中渲染它来断言 nonce 属性是否正确添加。
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/index.spec.tsx`:
- Around line 519-523: The test's selector for the CSS variable style is too
broad and matches usages like var(--primary-color); tighten it by searching for
the variable definition (e.g. include a colon) so you only find the style that
declares the CSS custom property; update the lookup for cssVarStyle (from
styles.find(...)) to check for a definition pattern such as '--primary-color:'
or a regex like /--primary-color\s*:/, then keep the existing
expect(cssVarStyle).toBeDefined() and expect(cssVarStyle?.nonce).toBe('bamboo')
assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f38cbe8b-d0ed-4358-9551-9e57bd1b7de3
📒 Files selected for processing (4)
src/StyleContext.tsxsrc/hooks/useCSSVarRegister.tssrc/hooks/useCacheToken.tsxtests/index.spec.tsx
| const styles = Array.from(document.head.querySelectorAll('style')); | ||
| // Box 组件使用 useCacheToken 注册 CSS var，应该有 nonce | ||
| const cssVarStyle = styles.find(s => s.innerHTML.includes('--primary-color')); | ||
| expect(cssVarStyle).toBeDefined(); | ||
| expect(cssVarStyle?.nonce).toBe('bamboo'); |
There was a problem hiding this comment.
CSS 变量样式定位条件过宽，断言可能误判。
当前查找条件会匹配到普通样式里的 var(--primary-color)，即使 CSS 变量样式本身没带 nonce，测试也可能通过。建议改为匹配变量定义（带冒号）的样式内容。
建议修改
- const cssVarStyle = styles.find(s => s.innerHTML.includes('--primary-color'));
+ const cssVarStyle = styles.find((s) =>
+ /--primary-color\s*:/.test(s.innerHTML),
+ );
expect(cssVarStyle).toBeDefined();
expect(cssVarStyle?.nonce).toBe('bamboo');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/index.spec.tsx` around lines 519 - 523, The test's selector for the CSS
variable style is too broad and matches usages like var(--primary-color);
tighten it by searching for the variable definition (e.g. include a colon) so
you only find the style that declares the CSS custom property; update the lookup
for cssVarStyle (from styles.find(...)) to check for a definition pattern such
as '--primary-color:' or a regex like /--primary-color\s*:/, then keep the
existing expect(cssVarStyle).toBeDefined() and
expect(cssVarStyle?.nonce).toBe('bamboo') assertions.
修复了当通过 StyleProvider 传递 nonce 时，CSS 变量样式未获得 nonce 属性的问题。 Changes: - 在 StyleContextProps 中添加 nonce 配置支持 - 在 useCacheToken 中应用 nonce 到 CSS var 样式注入 - 在 useCSSVarRegister 中应用 nonce 到 CSS var 样式注入 - 为 StyleProvider nonce 功能添加测试用例 用户现在可以一致地在所有样式注入点使用 StyleProvider 配置 nonce。 改进建议（未来优化）： - 提取 nonce 处理逻辑为共享函数以减少代码重复 - 当前测试已覆盖 useCacheToken → useCSSVarRegister 的调用路径
zombieJ
force-pushed
the
fix/css-var-nonce
branch
from
37fd7e5 to
0082730
Compare
提取了 useCacheToken 和 useCSSVarRegister 中重复的 nonce 处理逻辑为 mergeCSSConfig 共享函数，减少代码重复并提高可维护性。 Changes: - 在 util/index.ts 中添加 mergeCSSConfig 工具函数 - useCacheToken 使用 mergeCSSConfig 代替重复代码 - useCSSVarRegister 使用 mergeCSSConfig 代替重复代码
修复了 mergeCSSConfig 的泛型约束，使其支持 Options | undefined 类型，而不是只接受 Record<string, any>。 Changes: - 移除 mergeCSSConfig 的泛型约束 `T extends Record<string, any>` - 改为泛型 `T` 以支持任何类型
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/hooks/useCacheToken.tsx`:
- Around line 223-233: The TypeScript error comes from using Parameters<typeof
updateCSS>[2] which can be undefined; update the generic usage in both
useCacheToken.tsx and useCSSVarRegister.ts to wrap that type with NonNullable
(i.e., use NonNullable<Parameters<typeof updateCSS>[2]>) so mergeCSSConfig's
generic satisfies Record<string, any>; locate the mergeCSSConfig call in
useCacheToken where mergedCSSConfig is declared and the analogous merge call in
useCSSVarRegister around the noted lines and replace the generic type parameter
accordingly.
In `@src/util/index.ts`:
- Around line 158-176: The generic constraint on mergeCSSConfig (currently "T
extends Record<string, any>") is too strict for callers that pass
Parameters<typeof updateCSS>[2] (which may be undefined); change the generic
bound to allow undefined (e.g., "T extends Record<string, any> | undefined" or
remove the constraint entirely so T can be undefined) and keep the
implementation identical; update the function signature of mergeCSSConfig to use
the new generic constraint and ensure the return type still matches T;
alternatively callers can wrap the type with NonNullable, but the preferred fix
is to relax the constraint on mergeCSSConfig to accept undefined; references:
mergeCSSConfig, updateCSS, useCacheToken.tsx.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5a2414c-b7cc-41b4-8fe3-9e9f71cd5198
📒 Files selected for processing (3)
src/hooks/useCSSVarRegister.tssrc/hooks/useCacheToken.tsxsrc/util/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hooks/useCSSVarRegister.ts
| const mergedCSSConfig = mergeCSSConfig<Parameters<typeof updateCSS>[2]>( | ||
| { | ||
| mark: ATTR_MARK, | ||
| prepend: 'queue', | ||
| attachTo: container, | ||
| priority: -999, | ||
| }, | ||
| nonce, | ||
| ); | ||
|
|
||
| const style = updateCSS(cssVarsStr, hash(`css-var-${themeKey}`), mergedCSSConfig); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 首先查看 useCacheToken.tsx 文件的结构和大小
wc -l src/hooks/useCacheToken.tsx 2>/dev/null || find . -name "useCacheToken.tsx" -type fRepository: ant-design/cssinjs
Length of output: 93
🏁 Script executed:
# 读取 useCacheToken.tsx 的第 223-233 行
sed -n '223,233p' src/hooks/useCacheToken.tsxRepository: ant-design/cssinjs
Length of output: 387
🏁 Script executed:
# 查找 updateCSS 和 mergeCSSConfig 的定义
rg -n "export.*updateCSS|function updateCSS|const updateCSS" --type ts --type tsxRepository: ant-design/cssinjs
Length of output: 89
🏁 Script executed:
# 查找 mergeCSSConfig 的类型定义
rg -n "export.*mergeCSSConfig|function mergeCSSConfig|const mergeCSSConfig" --type ts --type tsxRepository: ant-design/cssinjs
Length of output: 89
🏁 Script executed:
# 检查 useCSSVarRegister.ts 中是否有相同的 mergeCSSConfig<Parameters 模式
rg -n "mergeCSSConfig<Parameters|mergeCSSConfig\s*<" src/hooks/useCSSVarRegister.ts --type ts --type tsx -A 3Repository: ant-design/cssinjs
Length of output: 89
🏁 Script executed:
# 查看 useCacheToken.tsx 的导入部分
head -n 30 src/hooks/useCacheToken.tsxRepository: ant-design/cssinjs
Length of output: 1108
🏁 Script executed:
# 搜索 updateCSS 定义（不用 --type）
rg -n "export.*updateCSS|function updateCSS|const updateCSS" -A 5Repository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 搜索 mergeCSSConfig 定义
rg -n "export.*mergeCSSConfig|function mergeCSSConfig|const mergeCSSConfig" -A 5Repository: ant-design/cssinjs
Length of output: 346
🏁 Script executed:
# 查找所有 TypeScript/TSX 文件中 updateCSS 的类型签名
rg "updateCSS.*=.*\(|function updateCSS\(" -A 3Repository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 在 src 目录查找相关文件
find src -type f \( -name "*.ts" -o -name "*.tsx" \) | head -20Repository: ant-design/cssinjs
Length of output: 600
🏁 Script executed:
# 查看 updateCSS 的调用方式和类型
rg -n "updateCSS\(" src/hooks/useCacheToken.tsx -B 5 -A 2Repository: ant-design/cssinjs
Length of output: 300
🏁 Script executed:
# 在 useCSSVarRegister.ts 中搜索相同的 mergeCSSConfig 模式
rg -n "mergeCSSConfig" src/hooks/useCSSVarRegister.ts -B 3 -A 5Repository: ant-design/cssinjs
Length of output: 761
🏁 Script executed:
# 查看 util/index.ts 中 mergeCSSConfig 的完整定义和使用
sed -n '161,180p' src/util/index.tsRepository: ant-design/cssinjs
Length of output: 533
🏁 Script executed:
# 搜索是否有 NonNullable 在项目中的使用
rg "NonNullable" -nRepository: ant-design/cssinjs
Length of output: 341
🏁 Script executed:
# 查看 package.json 以了解 `@rc-component/util` 的版本
cat package.json | grep -A 2 '"@rc-component/util"'Repository: ant-design/cssinjs
Length of output: 144
🏁 Script executed:
# 检查是否存在 TypeScript 编译错误日志或 tsconfig
find . -name "tsconfig.json" -o -name "*.d.ts" | grep -v node_modules | head -10Repository: ant-design/cssinjs
Length of output: 89
🏁 Script executed:
# 查看 useCSSVarRegister.ts 在第 74 行之前的导入
sed -n '1,25p' src/hooks/useCSSVarRegister.tsRepository: ant-design/cssinjs
Length of output: 904
🏁 Script executed:
# 检查 updateCSS 的类型定义（通过 `@rc-component/util`）
rg -n "export.*updateCSS" node_modules/@rc-component/util 2>/dev/null | head -5Repository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 尝试查看两个文件中 updateCSS 的使用上下文
rg -n "updateCSS" src/hooks/useCacheToken.tsx src/hooks/useCSSVarRegister.tsRepository: ant-design/cssinjs
Length of output: 717
🏁 Script executed:
# 检查项目是否有编译输出或类型检查结果
ls -la | grep -E "dist|build|lib"Repository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 查看 tsconfig 配置
cat tsconfig.json 2>/dev/null | head -30Repository: ant-design/cssinjs
Length of output: 568
🏁 Script executed:
# 确认 Parameters<typeof updateCSS>[2] 是否确实是可选的
# 通过查看 `@rc-component/util` 的类型定义
find . -path "*/node_modules/@rc-component/util*" -name "*.d.ts" | head -5Repository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 检查项目中是否已有编译错误
ls -la | grep -E "error|log"Repository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 看看 mergeCSSConfig 的实际使用方式
rg -n "mergeCSSConfig" src --context 5 -A 2Repository: ant-design/cssinjs
Length of output: 1260
🏁 Script executed:
# 检查 NonNullable 是否在项目中被使用过类似的方式
rg -n "NonNullable<Parameters" srcRepository: ant-design/cssinjs
Length of output: 44
🏁 Script executed:
# 验证两个文件中的 nonce 变量来源
sed -n '150,170p' src/hooks/useCacheToken.tsxRepository: ant-design/cssinjs
Length of output: 599
🏁 Script executed:
# 在 useCSSVarRegister.ts 中找 nonce 变量的来源
rg -n "nonce" src/hooks/useCSSVarRegister.ts -B 3 -A 1 | head -20Repository: ant-design/cssinjs
Length of output: 289
🏁 Script executed:
# 最终确认：查看两个文件中使用 mergeCSSConfig 的完整上下文
sed -n '220,235p' src/hooks/useCacheToken.tsxRepository: ant-design/cssinjs
Length of output: 492
🏁 Script executed:
# 同样查看 useCSSVarRegister.ts
sed -n '71,85p' src/hooks/useCSSVarRegister.tsRepository: ant-design/cssinjs
Length of output: 417
修复两个文件中的 TypeScript 类型错误 TS2344。
Parameters<typeof updateCSS>[2] 提取的第三个参数类型包含 undefined（因为该参数是可选的），但 mergeCSSConfig 的泛型约束要求 Record<string, any>，导致类型不兼容。需要用 NonNullable 包装来排除 undefined：
src/hooks/useCacheToken.tsx (第 223 行)
修复方案
- const mergedCSSConfig = mergeCSSConfig<Parameters<typeof updateCSS>[2]>(
+ const mergedCSSConfig = mergeCSSConfig<NonNullable<Parameters<typeof updateCSS>[2]>>(
{
mark: ATTR_MARK,
prepend: 'queue',
attachTo: container,
priority: -999,
},
nonce,
);src/hooks/useCSSVarRegister.ts (第 74 行)
修复方案
- const mergedCSSConfig = mergeCSSConfig<Parameters<typeof updateCSS>[2]>(
+ const mergedCSSConfig = mergeCSSConfig<NonNullable<Parameters<typeof updateCSS>[2]>>(
{
mark: ATTR_MARK,
prepend: 'queue',
attachTo: container,
priority: -999,
},
nonce,
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mergedCSSConfig = mergeCSSConfig<Parameters<typeof updateCSS>[2]>( | |
| { | |
| mark: ATTR_MARK, | |
| prepend: 'queue', | |
| attachTo: container, | |
| priority: -999, | |
| }, | |
| nonce, | |
| ); | |
| const style = updateCSS(cssVarsStr, hash(`css-var-${themeKey}`), mergedCSSConfig); | |
| const mergedCSSConfig = mergeCSSConfig<NonNullable<Parameters<typeof updateCSS>[2]>>( | |
| { | |
| mark: ATTR_MARK, | |
| prepend: 'queue', | |
| attachTo: container, | |
| priority: -999, | |
| }, | |
| nonce, | |
| ); | |
| const style = updateCSS(cssVarsStr, hash(`css-var-${themeKey}`), mergedCSSConfig); |
🧰 Tools
🪛 GitHub Actions: ✅ test
[error] 223-223: TypeScript error TS2344: Type 'Options | undefined' does not satisfy the constraint 'Record<string, any>'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useCacheToken.tsx` around lines 223 - 233, The TypeScript error
comes from using Parameters<typeof updateCSS>[2] which can be undefined; update
the generic usage in both useCacheToken.tsx and useCSSVarRegister.ts to wrap
that type with NonNullable (i.e., use NonNullable<Parameters<typeof
updateCSS>[2]>) so mergeCSSConfig's generic satisfies Record<string, any>;
locate the mergeCSSConfig call in useCacheToken where mergedCSSConfig is
declared and the analogous merge call in useCSSVarRegister around the noted
lines and replace the generic type parameter accordingly.
1 participant
Summary
修复了当通过
StyleProvider传递nonce时，CSS 变量（css var）样式标签未获得nonce属性的问题。现在nonce配置会在所有样式注入点（包括普通样式和 CSS var 样式）正确应用。Related Issue
Closes #57138 reported at ant-design/ant-design#57138
Background
useStyleRegister已正确处理nonce参数useCacheToken和useCSSVarRegister在注入 CSS var 样式时没有应用 nonceStyleContext缺少nonce配置项，导致StyleProvider无法统一管理 nonceChanges
StyleContextProps中添加nonce?: string | (() => string)配置useCacheToken: 从StyleContext读取 nonce 并传递给updateCSS的 csp 配置useCSSVarRegister: 从StyleContext读取 nonce 并传递给updateCSS的 csp 配置util/index.ts中添加mergeCSSConfig工具函数，提取公共的 nonce 处理逻辑StyleProvider nonce对 CSS var 样式的功能Refactoring
useCacheToken和useCSSVarRegister中重复的 nonce 处理代码为mergeCSSConfig共享函数Usage Example
Verification