feat: add CSP nonce support to Portal #35
Conversation
概览该PR为Portal和useScrollLocker组件添加了CSP nonce支持。新增可选的 变更
评估代码审查工作量🎯 2 (Simple) | ⏱️ ~12 分钟 诗
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 @juno-the-programmer, 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 enhances the 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. 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
|
![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 introduces Content Security Policy (CSP) nonce support to the Portal component, which is a valuable security enhancement. The changes are well-structured, maintaining backward compatibility for the useScrollLocker hook and including a comprehensive set of tests for the new functionality. I have one suggestion to improve the robustness and readability of the parameter parsing logic in useScrollLocker to prevent a potential runtime error.
| const options = typeof lock === 'object' ? lock : { lock }; | ||
| const mergedLock = !!(typeof lock === 'boolean' ? lock : options.lock); | ||
| const nonce = typeof lock === 'object' ? lock.nonce : undefined; |
There was a problem hiding this comment.
The current logic for parsing the lock parameter can be simplified for better readability and to handle null values gracefully. Currently, passing null as the lock argument would cause a runtime error because typeof null evaluates to 'object', leading to an attempt to access a property on a null value. This refactoring makes the code more robust and easier to understand.
| const options = typeof lock === 'object' ? lock : { lock }; | |
| const mergedLock = !!(typeof lock === 'boolean' ? lock : options.lock); | |
| const nonce = typeof lock === 'object' ? lock.nonce : undefined; | |
| const isOptions = typeof lock === 'object' && lock !== null; | |
| const mergedLock = !!(isOptions ? lock.lock : lock); | |
| const nonce = isOptions ? lock.nonce : undefined; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/useScrollLocker.tsx (1)
19-21: 参数处理逻辑正确，但可以简化。当前的参数规范化逻辑功能正确，兼容布尔值和选项对象两种形式。不过，第 20 行的逻辑稍显冗余，可以简化。
🔎 可选的简化建议
由于第 19 行已经将
options规范化，第 20 行可以直接使用options.lock：const options = typeof lock === 'object' ? lock : { lock }; - const mergedLock = !!(typeof lock === 'boolean' ? lock : options.lock); + const mergedLock = !!options.lock; const nonce = typeof lock === 'object' ? lock.nonce : undefined;这样代码更简洁，逻辑更清晰，同时保持相同的功能。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Portal.tsxsrc/useScrollLocker.tsxtests/index.test.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/Portal.tsx (1)
src/useScrollLocker.tsx (1)
useScrollLocker(16-52)
🔇 Additional comments (13)
src/Portal.tsx (3)
43-44: 类型定义和文档清晰准确。nonce 属性的类型定义正确，且 JSDoc 注释清楚地说明了其用途。
77-77: 属性解构正确。nonce 从 props 中正确解构。
123-130: 滚动锁定逻辑和 nonce 传递实现正确。shouldLock 的计算逻辑正确地考虑了所有必要条件（autoLock、open、DOM 可用性、容器类型），并且 nonce 正确地通过对象参数传递给 useScrollLocker。即使 nonce 为 undefined，useScrollLocker 也能正确处理。
src/useScrollLocker.tsx (3)
11-14: 接口定义清晰且类型正确。UseScrollLockerOptions 接口设计合理，两个可选属性的类型定义准确。
40-42: CSP 选项传递正确。只在 nonce 存在时才构造 CSP 对象，这种条件构造方式正确且高效，符合 updateCSS API 的预期格式。
51-51: 依赖数组完整且正确。useLayoutEffect 的依赖数组包含了所有在 effect 中使用的变量（mergedLock、id、nonce），确保在 nonce 变化时能够正确更新样式标签。
tests/index.test.tsx (7)
472-484: 基本 nonce 应用测试正确。测试用例正确验证了当 autoLock 启用时，nonce 属性被正确应用到样式标签上。断言清晰且符合预期行为。
486-497: 正确测试 autoLock 禁用时的行为。测试用例验证了当 autoLock 未启用时，即使传入了 nonce，样式标签也不会被创建。这是重要的边界情况。
499-530: 生命周期测试覆盖全面。测试用例完整验证了 Portal 的打开、关闭和重新打开过程中 nonce 的正确行为，确保样式标签的清理和重建都能正确处理 nonce 属性。
532-548: 自定义容器为 document.body 时的测试正确。测试用例验证了当通过 getContainer 显式指定 document.body 时，nonce 能够正确应用。这与 Portal 的 shouldLock 逻辑一致。
550-566: 自定义非 body 容器的测试正确。测试用例正确验证了当渲染到自定义非 body 容器时，不会应用滚动锁定和 nonce。这符合 Portal 的设计逻辑，并且正确清理了测试创建的 DOM 元素。
568-579: 向后兼容性测试重要且正确。测试用例验证了在不提供 nonce 的情况下，autoLock 功能仍然正常工作。这确保了现有代码的向后兼容性。
581-593: StrictMode 兼容性测试必要且正确。测试用例验证了在 React StrictMode 下 nonce 功能的正确性。这很重要，因为 StrictMode 会在开发环境中双重调用 effects，确保实现能够正确处理这种行为。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #35 +/- ##
==========================================
+ Coverage 94.57% 94.67% +0.09%
==========================================
Files 7 7
Lines 166 169 +3
Branches 54 57 +3
==========================================
+ Hits 157 160 +3
Misses 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@zombieJ please help to review thanks |
1 participant
This enables Portal to work in strict CSP environments by allowing developers to pass a nonce attribute for dynamically injected styles used in scroll locking.
Summary by CodeRabbit
发版说明
新功能
nonce属性，增强安全集成能力。测试
✏️ Tip: You can customize this high-level summary in your review settings.