-
Notifications
You must be signed in to change notification settings - Fork 263
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
feat: add plugin to remove rtl #2673
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Walkthrough本次更改引入了一个新的 Changes
Poem
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 @@
## next #2673 +/- ##
=======================================
Coverage 84.07% 84.07%
=======================================
Files 217 217
Lines 17830 17830
Branches 2608 2608
=======================================
Hits 14991 14991
Misses 2834 2834
Partials 5 5 ☔ View full report in Codecov by Sentry. |
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: 10
🧹 Outside diff range and nitpick comments (6)
packages/nutui-optimization-css/src/index.ts (1)
1-3
: 建议添加 JSDoc 文档说明
代码结构清晰简洁,作为入口文件重导出插件功能是个不错的设计。不过建议添加 JSDoc 文档来说明这个插件的用途和配置选项,这样可以为使用者提供更好的开发体验。
建议添加如下文档:
+/**
+ * PostCSS 插件,用于优化 CSS
+ * @param {Object} options - 配置选项
+ * @param {boolean} [options.removeRtl=false] - 是否移除 RTL 相关样式
+ * @param {Object} [options.cssVariables] - CSS 变量配置
+ * @param {string} [options.cssVariables.include] - 包含的 CSS 变量文件
+ * @param {string[]} [options.cssVariables.exclude] - 排除的 CSS 变量
+ * @param {string} [options.cssVariables.type] - CSS 变量替换策略
+ * @returns {import('postcss').Plugin} PostCSS 插件实例
+ */
import { profileGuidedOptimization } from './postcss-plugins'
export default profileGuidedOptimization
packages/nutui-optimization-css/build.config.ts (1)
15-17
: 考虑添加 source maps 支持
为了便于调试,建议添加 source maps 支持。这对于开发环境和问题排查都很有帮助。
建议在 rollup 配置中添加 source maps:
rollup: {
emitCJS: true,
+ sourceMap: true,
},
packages/nutui-optimization-css/README.md (3)
1-2
: 建议完善介绍部分的内容
当前介绍过于简单,建议补充以下内容:
- 具体的优化目标和收益
- 适用场景说明
- 支持的环境和版本要求
-通过配置 postcss 对组件库的 css 进行优化。
+# @nutui/opt-css
+
+通过配置 PostCSS 插件对组件库的 CSS 进行优化,主要功能包括:
+- 移除 RTL 相关样式,减小代码体积
+- 管理 CSS 变量,提供灵活的变量处理方案
+
+## 环境要求
+- PostCSS 8.0.0 及以上
+- Node.js 14.0.0 及以上
3-23
: 建议优化配置示例的可读性
配置示例可以增加注释说明,并提供更多使用场景的示例。
-1. Taro 环境下的配置示例
+## 使用示例
+
+### Taro 环境配置
+
+在 Taro 项目的配置文件中(如 `config/index.js`)添加以下配置:
```text
{
"mini": {
"postcss": {
+ // 启用 @nutui/opt-css 插件
"@nutui/opt-css": {
"enable": true,
"config": {
+ // 移除 RTL 相关样式
"removeRtl": true,
+ // CSS 变量配置
"cssVariables": {
+ // 指定包含 CSS 变量的文件
"include": [path.join(__dirname, 'variables.scss')],
+ // 排除不需要处理的 CSS 变量
"exclude": ["--nutui-color-primary-text"],
+ // 变量处理方式:replace 表示替换为实际值
"type": "replace"
}
}
}
}
}
}
+### 其他框架配置
+
+// TODO: 添加 webpack、vite 等其他框架的配置示例
---
`25-31`: **建议扩充配置说明部分**
当前配置说明较为简单,建议添加更详细的说明和使用示例。
```diff
-配置说明:
+## 配置说明
-1. removeRtl:删除 rtl 相关样式
-2. cssVariables
- - include: 指定css变量的文件
- - exclude: 设置哪些 css 变量不进行替换, 在 JS 控制 css 变量时可以使用 exclude 指定
- - type: css 变量的替换方案,默认不处理,当设置 replace 时,可将 css 变量替换为实际值
+### removeRtl
+
+用于删除 RTL(从右到左)相关的样式代码,可以显著减小样式文件体积。
+
+```css
+/* 优化前 */
+.nutui-button {
+ margin-left: 10px;
+ [dir='rtl'] & {
+ margin-right: 10px;
+ margin-left: 0;
+ }
+}
+
+/* 优化后 */
+.nutui-button {
+ margin-left: 10px;
+}
+```
+
+### cssVariables
+
+CSS 变量处理配置,提供灵活的变量管理方案。
+
+#### include
+指定包含 CSS 变量的文件路径,支持以下格式:
+- 单个文件:`'src/styles/variables.scss'`
+- 多个文件:`['src/styles/variables.scss', 'src/styles/themes.scss']`
+- glob 模式:`'src/styles/**/*.scss'`
+
+#### exclude
+设置不需要处理的 CSS 变量列表。常用于以下场景:
+- 需要通过 JavaScript 动态修改的主题变量
+- 需要保持响应式的变量
+
+示例:
+```javascript
+exclude: [
+ '--nutui-color-primary',
+ '--nutui-color-primary-text'
+]
+```
+
+#### type
+CSS 变量的处理方式:
+- `'none'`(默认):保持原样,不进行处理
+- `'replace'`:将 CSS 变量替换为实际值,可减小运行时开销
+
+示例:
+```css
+/* type: 'none' */
+.nutui-button {
+ color: var(--nutui-color-primary);
+}
+
+/* type: 'replace' */
+.nutui-button {
+ color: #2080f0;
+}
+```
packages/nutui-optimization-css/test/case.js (1)
5-18
: 建议扩展测试用例覆盖范围
当前的测试用例CSS不够全面,建议添加以下场景的测试:
- 嵌套的 RTL 规则
- 多个 CSS 变量在同一声明中的情况
- 不同类型的 CSS 选择器(ID、属性选择器等)
- 媒体查询中的 RTL 样式
建议添加如下测试用例:
@media screen and (max-width: 768px) {
[dir=rtl] .nut-mobile {
margin-left: 0;
margin-right: var(--nutui-spacing-4);
}
}
#nut-id[dir=rtl] {
padding: var(--nutui-padding-sm) var(--nutui-padding-lg);
}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
- packages/nutui-optimization-css/README.md (1 hunks)
- packages/nutui-optimization-css/build.config.ts (1 hunks)
- packages/nutui-optimization-css/package.json (1 hunks)
- packages/nutui-optimization-css/src/index.ts (1 hunks)
- packages/nutui-optimization-css/src/postcss-plugins/index.ts (1 hunks)
- packages/nutui-optimization-css/test/case.js (1 hunks)
- packages/nutui-optimization-css/test/variables.css (1 hunks)
- packages/nutui-optimization-css/test/variables.scss (1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/nutui-optimization-css/package.json
- packages/nutui-optimization-css/test/variables.css
- packages/nutui-optimization-css/test/variables.scss
🔇 Additional comments (1)
packages/nutui-optimization-css/README.md (1)
7-21
: 验证配置结构与实现的一致性
需要确保文档中的配置选项与实际实现保持一致。
✅ Verification successful
Let me gather more information about the implementation details.
配置结构与实现完全一致
文档中的配置选项与实际实现完全匹配:
removeRtl
选项支持布尔值cssVariables
配置支持:include
: 文件路径数组exclude
: CSS 变量名数组type
: 支持 'normal' | 'replace' 两种模式
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查实际实现中支持的配置选项
# 查找插件实现文件
echo "查找插件实现文件..."
rg -l "removeRtl|cssVariables" --type ts
# 检查配置项定义
echo "检查配置项定义..."
ast-grep --pattern 'interface $CONFIG {
$$$
removeRtl?: $_
cssVariables?: {
$$$
}
$$$
}'
Length of output: 498
Script:
#!/bin/bash
# 检查插件实现文件的具体内容
echo "检查 postcss-plugins/index.ts 的实现..."
cat packages/nutui-optimization-css/src/postcss-plugins/index.ts
# 查找配置类型定义
echo -e "\n查找配置类型定义..."
rg "type|interface.*Config" -A 10 packages/nutui-optimization-css/src/postcss-plugins/index.ts
Length of output: 3644
async function replaceCssVariables( | ||
root, | ||
cssVariablesContent: string[], |
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.
🛠️ Refactor suggestion
为函数参数添加类型注解
在异步函数 replaceCssVariables
中,参数 root
未指定类型。建议为其添加类型注解,例如 postcss.Root
,以增强类型检查和代码可读性。
修改如下:
- async function replaceCssVariables(
- root,
+ async function replaceCssVariables(
+ root: postcss.Root,
cssVariablesContent: string[],
exclude: string[] = []
) {
📝 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.
async function replaceCssVariables( | |
root, | |
cssVariablesContent: string[], | |
async function replaceCssVariables( | |
root: postcss.Root, | |
cssVariablesContent: string[], |
sourceFile.indexOf('@nutui') === -1 && | ||
sourceFile.indexOf('@dongdesign') === -1 |
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.
检查代码中 'dongdesign' 的引用是否正确
在第 25 行和第 26 行中,代码检查了 sourceFile
是否不包含 '@dongdesign'
。请确认这里的 '@dongdesign'
是否应替换为 '@nutui'
,以确保包名的一致性,避免混淆。
建议修改如下:
- sourceFile.indexOf('@dongdesign') === -1
+ sourceFile.indexOf('@nutui') === -1
Committable suggestion was skipped due to low confidence.
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: 10
🧹 Outside diff range and nitpick comments (4)
packages/nutui-optimize-css/test/remove-rtl.test.ts (1)
1-26
: 建议改进测试架构
为了提高测试的可维护性和可扩展性,建议:
- 创建专门的测试工具函数来处理 PostCSS 转换
- 将测试数据移至单独的文件
- 添加测试覆盖率检查
- 考虑添加性能基准测试
建议创建以下文件结构:
test/
├── __fixtures__/ # 测试数据
├── __snapshots__/ # 如果保留快照测试
├── utils/ # 测试工具函数
└── remove-rtl.test.ts
packages/nutui-optimize-css/test/replace-css-var.test.ts (1)
1-4
: 导入语句需要类型声明
建议为导入的模块添加类型声明,以增强类型安全性。
-import postcss from 'postcss'
+import type { Plugin } from 'postcss'
+import postcss from 'postcss'
packages/nutui-optimize-css/src/postcss-plugins/index.ts (2)
36-62
: 建议为 replaceCssVariables
函数的参数 root
指定类型
为了提高类型安全性和代码可读性,建议为 root
参数指定类型,例如 Root
。
可以应用以下修改:
-function replaceCssVariables(
- root,
+function replaceCssVariables(
+ root: Root,
cssVariablesContent: string[],
exclude: string[] = []
) {
// 函数主体保持不变
}
78-82
: 考虑在读取 CSS 变量文件失败时记录错误信息
当读取 CSS 变量文件失败时,代码默默地忽略了错误,这可能会使调试变得困难。建议在 catch
块中添加错误日志,以便更容易地定位问题。
可以应用以下修改:
} catch (e) {
+ console.error(`Failed to read CSS variable file: ${p}`, e)
content = ''
}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
packages/nutui-optimize-css/test/__snapshots__/remove-rtl.test.ts.snap
is excluded by!**/*.snap
packages/nutui-optimize-css/test/__snapshots__/replace-css-var.test.ts.snap
is excluded by!**/*.snap
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
- packages/nutui-optimize-css/README.md (1 hunks)
- packages/nutui-optimize-css/build.config.ts (1 hunks)
- packages/nutui-optimize-css/package.json (1 hunks)
- packages/nutui-optimize-css/src/index.ts (1 hunks)
- packages/nutui-optimize-css/src/postcss-plugins/index.ts (1 hunks)
- packages/nutui-optimize-css/test/remove-rtl.test.ts (1 hunks)
- packages/nutui-optimize-css/test/replace-css-var.test.ts (1 hunks)
- packages/nutui-optimize-css/test/variables.css (1 hunks)
- packages/nutui-optimize-css/test/variables.scss (1 hunks)
- packages/nutui-optimize-css/tsconfig.json (1 hunks)
- packages/nutui-optimize-css/vitest.config.ts (1 hunks)
✅ Files skipped from review due to trivial changes (8)
- packages/nutui-optimize-css/README.md
- packages/nutui-optimize-css/build.config.ts
- packages/nutui-optimize-css/package.json
- packages/nutui-optimize-css/src/index.ts
- packages/nutui-optimize-css/test/variables.css
- packages/nutui-optimize-css/test/variables.scss
- packages/nutui-optimize-css/tsconfig.json
- packages/nutui-optimize-css/vitest.config.ts
import postcss from 'postcss' | ||
import { describe, expect, it } from 'vitest' | ||
import optimizeCss from '../dist/index.cjs' |
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.
建议从源代码而不是构建文件中导入
建议将导入路径从 '../dist/index.cjs' 改为 '../src/index',这样可以:
- 确保测试始终使用最新的源代码
- 避免在运行测试前必须先构建项目
- 提高测试的可靠性
import postcss from 'postcss'
import { describe, expect, it } from 'vitest'
-import optimizeCss from '../dist/index.cjs'
+import optimizeCss from '../src/index'
Committable suggestion was skipped due to low confidence.
describe('@nutui/optimize-css', () => { | ||
it('optimize css', async () => { | ||
const a = await postcss([ | ||
optimizeCss({ | ||
cssVariables: { | ||
include: [path.join(__dirname, 'variables.scss')], | ||
exclude: ['--nutui-color-primary-text'], | ||
type: 'replace', | ||
}, | ||
}), | ||
]).process(css, { from: undefined }) | ||
const optimizedCsss = a.css.toString() | ||
console.log(optimizedCsss) | ||
// @ts-ignore | ||
expect(optimizedCsss).toMatchSnapshot() | ||
}) | ||
}) |
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.
💡 Codebase verification
测试覆盖率不足的问题已确认,建议扩展测试用例
根据源码实现,当前确实缺少以下关键测试场景:
- CSS变量替换模式(type: 'normal' vs 'replace')的对比测试
- include 文件不存在时的容错测试
- 多个 CSS 变量嵌套使用的场景测试
- preserve 选项对 exclude 变量的保留验证
建议参考以下测试结构添加用例:
describe('@nutui/optimize-css', () => {
describe('CSS变量替换', () => {
it('normal 模式应保留所有变量', async () => {})
it('replace 模式应正确替换非 exclude 变量', async () => {})
it('处理不存在的 include 文件时不应报错', async () => {})
it('正确处理嵌套的 CSS 变量', async () => {})
it('preserve 应正确保留被 exclude 的变量', async () => {})
})
})
🔗 Analysis chain
建议添加更多测试场景
当前测试覆盖率不够全面,建议添加以下测试场景:
- CSS 变量替换的边界情况
- exclude 配置的有效性验证
- 不同 type 配置的行为验证
建议参考以下测试结构:
describe('@nutui/optimize-css', () => {
describe('CSS 变量替换', () => {
// 现有的基础测试用例
// 边界情况测试
// exclude 配置测试
// type 配置测试
})
})
Let me check the test files content to better understand the current test coverage.
Let me check the implementation file in a different way since the previous command didn't work.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查当前测试覆盖率
rg -l "describe|it\(" packages/nutui-optimize-css/test/
Length of output: 166
Script:
#!/bin/bash
# Check content of test files
echo "=== replace-css-var.test.ts ==="
cat packages/nutui-optimize-css/test/replace-css-var.test.ts
echo -e "\n=== remove-rtl.test.ts ==="
cat packages/nutui-optimize-css/test/remove-rtl.test.ts
# Check implementation file to understand what scenarios need to be tested
echo -e "\n=== Implementation file ==="
fd "optimize-css" packages/nutui-optimize-css/src/ --type f --exec cat {}
Length of output: 1900
Script:
#!/bin/bash
# Try to find and check the implementation file
echo "=== Source files ==="
find packages/nutui-optimize-css/src -type f -exec cat {} \;
# Also check if there's a variables.scss file
echo -e "\n=== variables.scss ==="
cat packages/nutui-optimize-css/test/variables.scss
Length of output: 2903
# Conflicts: # pnpm-lock.yaml
通过配置 postcss 对组件库的 css 进行优化。
配置说明:
Summary by CodeRabbit
@nutui/optimize-css
包,提供 CSS 优化功能。