-
Notifications
You must be signed in to change notification settings - Fork 266
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: icons 的替换方案优化,支持无法可用 icon 映射的情况 #2922
Conversation
变更概览走查这个拉取请求主要修改了 变更
可能相关的 PR
建议标签
诗歌
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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 #2922 +/- ##
==========================================
Coverage 85.97% 85.97%
==========================================
Files 277 277
Lines 18692 18692
Branches 2769 2769
==========================================
Hits 16070 16070
Misses 2617 2617
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: 0
🧹 Nitpick comments (5)
pnpm-workspace.yaml (1)
1-5
: 建议修复文件格式问题YAML 文件末尾缺少换行符,这可能会在某些工具中导致警告。
建议应用以下修改:
- 'packages/nutui-codemod' - 'packages/nutui-auto-import' - 'packages/nutui-taro-demo' - - 'packages/nutui-replace-icons' + - 'packages/nutui-replace-icons' +🧰 Tools
🪛 yamllint (1.35.1)
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
packages/nutui-replace-icons/src/replace-icons.ts (4)
15-18
: 判断导入源库与跳过集合此处通过校验
sourceLibraries
和skip
集合来确保只处理指定库且避免重复操作。可以考虑抽取至辅助函数,方便后续扩展或测试。
19-46
: 在 try 块中构建新的导入声明通过遍历 specifier 构建全新的 ImportDeclaration 并插入到 Program 节点,然后移除旧节点,逻辑完整。建议拆分为更小的函数,提高可读性与维护性,也可在捕获到错误时添加更多上下文信息。
🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
31-34
: 可使用可选链来简化对 iconMappings 的判断可以将
iconMappings && iconMappings[importedIdentifier]
更改为iconMappings?.[importedIdentifier]
,使代码更简洁并避免潜在的空值异常。🧰 Tools
🪛 Biome (1.9.4)
[error] 32-32: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
53-54
: 日志中仅打印错误信息当前只在控制台输出错误,若在实际工程中使用,建议集成更完善的日志或监控系统,便于后续追踪与分析。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/nutui-replace-icons/test/__snapshots__/exclude-case-inline.test.ts.snap
is excluded by!**/*.snap
packages/nutui-replace-icons/test/__snapshots__/exclude-case.test.ts.snap
is excluded by!**/*.snap
packages/nutui-replace-icons/test/__snapshots__/replace-case.test.ts.snap
is excluded by!**/*.snap
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
packages/nutui-replace-icons/src/replace-icons.ts
(1 hunks)packages/nutui-replace-icons/src/type.ts
(1 hunks)packages/nutui-replace-icons/test/exclude-case-inline.test.ts
(1 hunks)packages/nutui-replace-icons/test/exclude-case.test.ts
(1 hunks)packages/nutui-replace-icons/test/replace-case.test.ts
(1 hunks)pnpm-workspace.yaml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
pnpm-workspace.yaml
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
🪛 Biome (1.9.4)
packages/nutui-replace-icons/src/replace-icons.ts
[error] 32-32: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (15)
pnpm-workspace.yaml (1)
5-5
: 新增包路径符合项目结构!新增的
nutui-replace-icons
包路径与 PR 的目标(优化图标替换方案)保持一致,遵循了工作区的命名规范。🧰 Tools
🪛 yamllint (1.35.1)
[error] 5-5: no new line character at the end of file
(new-line-at-end-of-file)
packages/nutui-replace-icons/src/replace-icons.ts (4)
3-5
: 新增了 ReplaceOptions 接口通过继承 IOptions 并自定义
exclude
字段,对于灵活管理排除项非常有用。
7-7
: 使用全局 skip 集合这里维护一个全局的
skip
集合能避免重复处理相同节点,但需要留意多文件或多次执行时可能的状态残留,建议在更适合的作用域进行管理。
9-12
: replace 函数签名变更将参数类型切换为
ReplaceOptions
后,更好地封装了排除逻辑与库信息,结构更加清晰可维护。
71-76
: 将 exclude 数组转换为哈希表通过遍历
exclude
数组生成excludeHashMap
,可在后面的替换逻辑中快速判断是否需要排除,方式简洁高效。packages/nutui-replace-icons/src/type.ts (1)
5-5
: 在 IOptions 中新增可选的 exclude 字段新增了
exclude?: Array<string>
,使得替换功能可配置需要排除的图标,增加使用灵活度。packages/nutui-replace-icons/test/replace-case.test.ts (1)
7-7
: 显式指定 sourceLibrary在测试配置中增加
sourceLibrary: ['@nutui/icons-react']
,可确保只替换特定来源的图标,减少误替换风险。packages/nutui-replace-icons/test/exclude-case-inline.test.ts (4)
5-12
: 新增 exclude 配置此处将
Loading1
和ArrowSize6
放入 exclude 列表,以验证替换过程不会影响已列入排除的图标。
14-17
: Babel 配置中的插件将
plugin
注入 Babel 的插件数组,用来测试代码转换流程,方便后续断言转换结果。
19-23
: 测试用示例代码导入多个图标并在 JSX 中使用,为快照测试提供可观测的变化场景,便于验证排除与替换逻辑。
24-30
: 使用 Vitest 进行快照测试通过快照断言可直观地确认图标是否被正确替换或排除,是较常见而有效的验证手段。
packages/nutui-replace-icons/test/exclude-case.test.ts (4)
5-12
: 排除指定图标的测试配置在
replaceIcons
插件中同时配置了iconMappings
与exclude
,可以充分验证同一文件中既有要替换的图标也有要排除的图标时的行为。
14-17
: Babel 配置注入类似其他测试用例,将插件传入 Babel config 并执行转换,保证测试用例一致性。
19-24
: 示例代码导入多个图标同时导入被替换的图标(Loading)和被排除的图标(Loading1, ArrowSize6),测试覆盖更全面。
26-32
: Vitest 快照对比通过快照测试,能清晰对比转换前后是否达到预期替换与排除效果,推荐保留。
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 (1)
package.json (1)
66-67
: 建议:更新构建流程文档构建流程发生了变更,建议更新相关文档以反映新增的 CSS 变量替换步骤。
建议在项目文档中添加以下内容:
- 新增构建步骤的目的和作用
replace-css-var.js
脚本的功能说明- 构建流程的完整执行顺序
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (1)
package.json (1)
66-67
: 验证新增的构建脚本构建流程中新增了 CSS 变量替换步骤,需要确保
replace-css-var.js
脚本的实现正确且可用。运行以下脚本验证新文件的存在和权限:
✅ Verification successful
构建脚本验证完成
replace-css-var.js
脚本已完成验证:
- 脚本文件存在且权限正确
- 实现逻辑完整,用于处理组件 CSS 变量
- 所有引用的配置文件和样式文件均存在
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 验证新脚本文件是否存在且具有执行权限 if [ -f "./scripts/replace-css-var.js" ]; then echo "文件存在" ls -l "./scripts/replace-css-var.js" else echo "错误:找不到 replace-css-var.js 文件" exit 1 fi # 检查脚本的实现 echo "脚本内容预览:" head -n 20 "./scripts/replace-css-var.js"Length of output: 933
Script:
#!/bin/bash # 验证依赖项 echo "检查 package.json 中的依赖项:" grep -A 5 '"dependencies"' package.json grep -A 5 '"devDependencies"' package.json # 验证引用的文件 echo -e "\n检查引用的文件:" ls -l src/config.json ls -l src/styles/mixins/text-ellipsis.scss ls -l src/styles/variables.scssLength of output: 974
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
测试
改进