refactor: site running faster when it's development #2926
Conversation
概述遍历本次拉取请求包含多个文件的修改，主要涉及代码清理、性能优化和组件结构调整。更改包括移除注释代码、简化导入和导出、优化组件渲染逻辑，以及更新URL和资源加载方式。这些修改旨在提高代码可读性和组件性能。 变更
建议的审阅者
标签
诗歌
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2926 +/- ##
=============================================
- Coverage 85.97% 85.58% -0.40%
=============================================
Files 277 277
Lines 18692 18185 -507
Branches 2769 2763 -6
=============================================
- Hits 16070 15563 -507
Misses 2617 2617
Partials 5 5 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/sites/doc/App.tsx (1)
Line range hint
45-47: 添加事件监听器的清理函数在 useEffect 中需要清理 scroll 事件监听器，以防止内存泄漏：
useEffect(() => { document.addEventListener('scroll', scrollTitle) + return () => { + document.removeEventListener('scroll', scrollTitle) + } }, [])
🧹 Nitpick comments (5)
src/sites/assets/locale/index.ts (1)
14-16: 建议添加性能改进的注释说明为了帮助其他开发者理解这个性能优化的目的，建议添加注释说明为什么移除了动态更新功能。
+ // 性能优化：语言包只在初始化时加载一次，避免不必要的重渲染 const [translated] = useState<BaseLang & T>( languagesPackage[locale || 'zh-CN'] as any )src/sites/assets/locale/taro/index.ts (1)
1-1: 代码保持一致性，建议统一文档化Taro 版本的改动与 Web 版本保持一致，这很好。建议：
- 确保两个文件的改动保持同步
- 在项目文档中说明这个性能优化的细节
建议在项目的 README 或开发文档中添加以下说明：
## 性能优化说明 - 语言包加载策略：为提高开发环境性能，语言包仅在组件初始化时加载一次 - 适用范围：Web 与 Taro 版本均采用此优化 - 使用限制：不支持运行时动态切换语言Also applies to: 14-16
src/sites/doc/App.tsx (1)
Line range hint
15-29: 建议将 Title 组件提取到单独的文件中Title 组件应该被移动到它自己的文件中，这样可以：
- 提高代码的组织性
- 使组件更容易测试
- 遵循单一职责原则
src/packages/button/button.tsx (1)
Line range hint
73-89: 优化 getStyle 的性能和类型安全性
getStyle的实现有以下改进空间：
- props.fill 应该从解构的 fill 参数中获取
- 颜色检查逻辑可以更严格
const getStyle = useMemo(() => { const style: CSSProperties = {} if (color) { - if (props.fill === 'outline' || props.fill === 'dashed') { + if (fill === 'outline' || fill === 'dashed') { style.color = color - if (!color?.includes('gradient')) { + if (typeof color === 'string' && !color.includes('gradient')) { style.borderColor = color } } else { style.color = '#fff' style.background = color style.borderColor = 'transparent' } } return style -}, [color, props.fill]) +}, [color, fill])src/packages/button/button.taro.tsx (1)
127-145: 代码结构优化：类名生成逻辑重构将类名生成逻辑抽取为独立变量提高了代码的可读性和维护性。建议考虑进一步优化：
- const buttonClassNames = classNames( + const getButtonClassNames = useMemo(() => classNames( prefixCls, `${prefixCls}-${type}`, // ... other class names className - ) + ), [type, fill, size, shape, block, disabled, loading, className])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
index.html(4 hunks)src/config.json(0 hunks)src/packages/button/button.taro.tsx(7 hunks)src/packages/button/button.tsx(6 hunks)src/sites/assets/locale/index.ts(2 hunks)src/sites/assets/locale/taro/index.ts(2 hunks)src/sites/assets/styles/md-style.scss(0 hunks)src/sites/assets/styles/variables.scss(0 hunks)src/sites/assets/util/index.ts(1 hunks)src/sites/doc/App.scss(0 hunks)src/sites/doc/App.tsx(1 hunks)src/sites/doc/components/demo-preview/demo-preview.scss(0 hunks)src/sites/doc/components/demo-preview/demo-preview.tsx(1 hunks)src/sites/doc/components/demoblock/codeblock.tsx(0 hunks)src/sites/doc/components/header/header.scss(0 hunks)src/sites/doc/components/header/header.tsx(0 hunks)src/sites/doc/components/issue/issue.scss(0 hunks)src/sites/doc/components/issue/issue.tsx(1 hunks)src/sites/doc/components/nav/nav.tsx(0 hunks)src/sites/mobile/App.tsx(2 hunks)theme.html(3 hunks)
💤 Files with no reviewable changes (10)
- src/sites/assets/styles/md-style.scss
- src/sites/doc/components/demo-preview/demo-preview.scss
- src/sites/doc/components/header/header.tsx
- src/sites/assets/styles/variables.scss
- src/sites/doc/components/issue/issue.scss
- src/sites/doc/components/demoblock/codeblock.tsx
- src/sites/doc/components/header/header.scss
- src/sites/doc/components/nav/nav.tsx
- src/config.json
- src/sites/doc/App.scss
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (11)
src/sites/doc/components/demo-preview/demo-preview.tsx (1)
15-15: 改进了无障碍性，但需要注意安全性和性能问题为iframe添加title属性提高了可访问性，这是一个很好的改进。
然而，有以下几点建议：
- 建议添加sandbox属性以提高安全性：
- <iframe title="demos" src={`/react/demo.html#${URL}`}></iframe> + <iframe + title="demos" + src={`/react/demo.html#${URL}`} + sandbox="allow-scripts allow-same-origin" + ></iframe>
- 为提高性能，建议添加懒加载：
- <iframe title="demos" src={`/react/demo.html#${URL}`}></iframe> + <iframe + title="demos" + src={`/react/demo.html#${URL}`} + loading="lazy" + ></iframe>
- 请验证URL路径的安全性：
✅ Verification successful
URL 使用安全性验证已完成，建议优化性能
经过验证，URL 的使用方式是安全的，因为：
- 路径参数通过 URL hash 传递，不会造成服务器端安全问题
- 遵循了 React Router 的标准路由实践
为了优化性能，建议添加懒加载：
- <iframe title="demos" src={`/react/demo.html#${URL}`}></iframe> + <iframe + title="demos" + src={`/react/demo.html#${URL}`} + loading="lazy" + ></iframe>🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查location.pathname的使用和验证逻辑 rg -A 5 "location.pathname" src/Length of output: 4223
src/sites/doc/components/issue/issue.tsx (1)
5-5: 代码重构改进了可维护性将
getComponentName函数移至工具文件是个很好的重构，提高了代码的可重用性和可维护性。src/packages/button/button.tsx (2)
90-97: handleClick 的实现很好地利用了 useCallback使用 useCallback 和正确的依赖数组优化了点击处理函数的性能。
99-116: buttonClassNames 的提取提高了代码可读性将类名逻辑提取到单独的变量中是个很好的改进，使代码更清晰易读。
src/sites/mobile/App.tsx (2)
4-4: 导入优化和代码重构通过将
getComponentName函数移至公共工具模块并简化loadable导入，提高了代码的可维护性和复用性。Also applies to: 11-11
83-83: 代码清理和性能优化移除了本地的
getComponentName实现，改用集中式的实现，这样可以减少代码重复并提高性能。src/packages/button/button.taro.tsx (2)
Line range hint
84-103: 性能优化：使用 useMemo 优化样式计算将样式计算逻辑从
useCallback改为useMemo是一个很好的优化，因为：
- 样式计算是纯计算操作，更适合使用
useMemo- 依赖项的设置更加准确，避免不必要的重计算
118-125: 优化：事件处理使用 useCallback使用
useCallback包装点击处理函数是一个很好的实践：
- 避免不必要的函数重建
- 正确处理了 loading 和 disabled 状态
index.html (2)
7-13: 性能优化：添加 DNS 预解析添加 DNS 预解析是一个很好的性能优化：
- 减少 DNS 解析时间
- 提高页面加载速度
5-5: 更新：技术栈变更和资源优化
- 更新了标题以反映从 Vue 到 React 的变更
- 使用协议相对 URL 提高了灵活性
- 优化了脚本加载方式
Also applies to: 52-52, 82-82
theme.html (1)
7-13: 性能优化：DNS 预解析配置与 index.html 保持一致的 DNS 预解析配置，确保了整站的性能优化策略统一。
| @@ -1,4 +1,4 @@ | |||
| import React, { useEffect, useState } from 'react' | |||
| import React, { useState } from 'react' | |||
There was a problem hiding this comment.
💡 Codebase verification
需要恢复动态语言切换功能
代码分析显示系统确实需要支持动态语言切换：
useLocalehook 实现了基于 URL 的语言检测和切换功能- 支持通过
popstate事件响应 URL 变化 - Taro 版本也预留了语言切换的能力
建议：
- 恢复
useEffect的使用 - 保留
setLanguagesPackage状态更新函数 - 确保语言包能够随 URL 变化而更新
🔗 Analysis chain
性能优化的权衡取舍需要确认
移除 useEffect 和状态更新函数确实能提高性能，但这意味着语言包在组件生命周期内将保持不变。请确认：
- 用户是否需要在运行时动态切换语言？
- 语言包是否会在运行时更新？
如果以上场景都不需要支持，那么这个优化是合理的。
Also applies to: 14-16
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否存在运行时切换语言的功能
rg -l "setLocale|changeLocale|switchLocale"
Length of output: 124
Script:
#!/bin/bash
# Check implementation of locale switching
echo "=== Main uselocale.ts ==="
cat src/sites/assets/locale/uselocale.ts
echo -e "\n=== Taro uselocale.ts ==="
cat src/sites/assets/locale/taro/uselocale.ts
Length of output: 1314
| const getComponentName = () => { | ||
| const s = window.location.hash.split('/') | ||
| const cname = s[s.length - 1].toLowerCase() | ||
| const component: any = {} | ||
| nav.forEach((item: any) => { | ||
| item.packages.forEach((sItem: any) => { | ||
| if (sItem.name.toLowerCase() == cname) { | ||
| component.name = sItem.name | ||
| component.cName = sItem.cName | ||
| return | ||
| } | ||
| }) | ||
| }) | ||
| return component | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
改进 getComponentName 函数的类型安全性和错误处理
建议进行以下改进：
- 避免使用
any类型，应该为 nav 项定义明确的接口 - 添加错误处理，当找不到匹配项时返回默认值
- 考虑使用可选链操作符来避免潜在的空值错误
-const getComponentName = () => {
+interface NavItem {
+ name: string
+ cName: string
+}
+
+interface NavPackage {
+ packages: NavItem[]
+}
+
+const getComponentName = (): { name: string; cName: string } => {
const s = window.location.hash.split('/')
const cname = s[s.length - 1].toLowerCase()
- const component: any = {}
- nav.forEach((item: any) => {
- item.packages.forEach((sItem: any) => {
+ const component = { name: '', cName: '' }
+ nav.forEach((item: NavPackage) => {
+ item.packages?.forEach((sItem: NavItem) => {
if (sItem.name.toLowerCase() == cname) {
component.name = sItem.name
component.cName = sItem.cName
return
}
})
})
return component
}📝 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 getComponentName = () => { | |
| const s = window.location.hash.split('/') | |
| const cname = s[s.length - 1].toLowerCase() | |
| const component: any = {} | |
| nav.forEach((item: any) => { | |
| item.packages.forEach((sItem: any) => { | |
| if (sItem.name.toLowerCase() == cname) { | |
| component.name = sItem.name | |
| component.cName = sItem.cName | |
| return | |
| } | |
| }) | |
| }) | |
| return component | |
| } | |
| interface NavItem { | |
| name: string | |
| cName: string | |
| } | |
| interface NavPackage { | |
| packages: NavItem[] | |
| } | |
| const getComponentName = (): { name: string; cName: string } => { | |
| const s = window.location.hash.split('/') | |
| const cname = s[s.length - 1].toLowerCase() | |
| const component = { name: '', cName: '' } | |
| nav.forEach((item: NavPackage) => { | |
| item.packages?.forEach((sItem: NavItem) => { | |
| if (sItem.name.toLowerCase() == cname) { | |
| component.name = sItem.name | |
| component.cName = sItem.cName | |
| return | |
| } | |
| }) | |
| }) | |
| return component | |
| } |
| iconUrl: "https://nutui.jd.com/img/logo_share.png", | ||
| url: "https://nutui.jd.com/demo.html#/index", | ||
| title: "轻量级移动端Vue组件库 - NutUI 3.0", | ||
| desc: "京东风格的Vue组件库", |
There was a problem hiding this comment.
需要更新：分享配置中的描述
分享配置中仍然引用了 Vue 相关的描述，需要更新：
var shareOption = {
iconUrl: "https://nutui.jd.com/img/logo_share.png",
url: "https://nutui.jd.com/demo.html#/index",
- title: "轻量级移动端Vue组件库 - NutUI 3.0",
- desc: "京东风格的Vue组件库",
+ title: "轻量级移动端React组件库 - NutUI",
+ desc: "京东风格的React组件库",
};📝 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.
| iconUrl: "https://nutui.jd.com/img/logo_share.png", | |
| url: "https://nutui.jd.com/demo.html#/index", | |
| title: "轻量级移动端Vue组件库 - NutUI 3.0", | |
| desc: "京东风格的Vue组件库", | |
| iconUrl: "https://nutui.jd.com/img/logo_share.png", | |
| url: "https://nutui.jd.com/demo.html#/index", | |
| title: "轻量级移动端React组件库 - NutUI", | |
| desc: "京东风格的React组件库", |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/sites/doc/components/demoblock/codeblock.tsx (1)
7-12: 建议考虑使用懒加载来进一步提升性能虽然将 glob 模式限制为 h5 目录下的演示文件是个好的改进，但是
eager: true的设置会强制立即加载所有匹配的模块。考虑使用懒加载可能会带来更好的性能提升。const modules = import.meta.glob(`@/packages/**/demos/h5/*.tsx`, { query: '?raw', import: 'default', - eager: true, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/sites/doc/components/demoblock/codeblock.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/sites/doc/components/demoblock/codeblock.tsx
[error] 21-21: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
| try { | ||
| const highlightedCode = hljs.highlightAuto(originCode, ['jsx']).value | ||
| return ( | ||
| <DemoBlock text={originCode} scss=""> | ||
| <pre> | ||
| <code dangerouslySetInnerHTML={{ __html: highlightedCode }} /> | ||
| </pre> | ||
| </DemoBlock> | ||
| ) | ||
| } catch(e) { | ||
| console.log('e', e) | ||
| return <></> | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议改进错误处理和用户反馈
当前的错误处理方式会导致用户界面静默失败，建议添加用户友好的错误提示。
} catch(e) {
console.log('e', e)
- return <></>
+ return (
+ <div className="code-error-message">
+ <p>代码加载失败，请刷新页面重试</p>
+ <p className="error-details">{e.message}</p>
+ </div>
+ )
}📝 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.
| try { | |
| const highlightedCode = hljs.highlightAuto(originCode, ['jsx']).value | |
| return ( | |
| <DemoBlock text={originCode} scss=""> | |
| <pre> | |
| <code dangerouslySetInnerHTML={{ __html: highlightedCode }} /> | |
| </pre> | |
| </DemoBlock> | |
| ) | |
| } catch(e) { | |
| console.log('e', e) | |
| return <></> | |
| } | |
| try { | |
| const highlightedCode = hljs.highlightAuto(originCode, ['jsx']).value | |
| return ( | |
| <DemoBlock text={originCode} scss=""> | |
| <pre> | |
| <code dangerouslySetInnerHTML={{ __html: highlightedCode }} /> | |
| </pre> | |
| </DemoBlock> | |
| ) | |
| } catch(e) { | |
| console.log('e', e) | |
| return ( | |
| <div className="code-error-message"> | |
| <p>代码加载失败，请刷新页面重试</p> | |
| <p className="error-details">{e.message}</p> | |
| </div> | |
| ) | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
| <DemoBlock text={originCode} scss=""> | ||
| <pre> | ||
| <code dangerouslySetInnerHTML={{ __html: highlightedCode }} /> | ||
| </pre> |
There was a problem hiding this comment.
💡 Codebase verification
建议：代码高亮输出需要进行 HTML 净化处理
为了防止潜在的 XSS 攻击风险，建议采取以下安全措施：
- 安装 DOMPurify 库：
npm install dompurify
npm install --save-dev @types/dompurify # TypeScript 类型支持- 修改代码实现：
import DOMPurify from 'dompurify';
<code dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(highlightedCode) }} />即使 highlightedCode 来自可信的代码高亮库，添加 HTML 净化层也是一个好的安全实践，可以防止将来可能引入的安全漏洞。
🔗 Analysis chain
注意：使用 dangerouslySetInnerHTML 存在安全风险
直接使用 dangerouslySetInnerHTML 渲染代码可能存在 XSS 攻击风险。建议在设置 HTML 内容之前对 highlightedCode 进行安全过滤。
建议使用 DOMPurify 库来净化 HTML 内容：
+import DOMPurify from 'dompurify'
- <code dangerouslySetInnerHTML={{ __html: highlightedCode }} />
+ <code dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(highlightedCode) }} />运行以下命令检查是否已安装 DOMPurify：
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 package.json 中是否包含 DOMPurify
cat package.json | jq '.dependencies["dompurify"] // .devDependencies["dompurify"]'
Length of output: 90
Script:
#!/bin/bash
# Search for syntax highlighter imports and highlightedCode generation
rg -l "highlight" src/sites/doc/components/demoblock/ --type ts --type tsx
ast-grep --pattern 'import $_ from "$_"' src/sites/doc/components/demoblock/codeblock.tsx
Length of output: 194
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
发布说明
新功能
代码优化
样式调整
性能改进
useMemo和useCallback优化了组件渲染其他变更