-
Notifications
You must be signed in to change notification settings - Fork 262
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 replace icons #2671
Conversation
Warning Rate limit exceeded@oasis-cloud has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough在此拉取请求中, Changes
Possibly related PRs
Suggested labels
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 #2671 +/- ##
=======================================
Coverage 84.07% 84.07%
=======================================
Files 217 217
Lines 17829 17829
Branches 2608 2608
=======================================
Hits 14990 14990
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: 8
🧹 Outside diff range and nitpick comments (6)
packages/nutui-replace-icons/src/type.ts (1)
1-5
: 接口结构清晰,建议添加文档注释
接口定义基本合理,但建议添加以下改进:
- 添加 JSDoc 注释来说明每个属性的用途和预期值
- 考虑为
targetLibrary
添加路径格式验证
建议添加如下文档注释:
+/**
+ * 图标替换插件的配置选项
+ */
export interface IOptions {
+ /** 源图标库路径列表 */
sourceLibrary: string[]
+ /** 目标图标库路径 */
targetLibrary: string
+ /** 可选的图标映射关系 */
iconMappings?: { [key: string]: string }
}
packages/nutui-replace-icons/test/replace-case.test.ts (1)
12-15
: Babel 配置正确,建议小改进
配置内容本身没有问题,但建议使用 @babel/core
提供的类型定义来增强类型安全性。
+import { TransformOptions } from '@babel/core';
-const babelOptions = {
+const babelOptions: TransformOptions = {
presets: ['@babel/preset-react'],
plugins: [plugin],
}
packages/nutui-replace-icons/README.md (3)
23-31
: 配置示例需要改进
配置对象的结构不够清晰,建议:
- 添加对
targetIconLibary
和iconMap
配置项的详细说明 - 列出支持的图标库列表
- 优化代码格式,保持一致的缩进
建议修改为:
[
- repleaceIcons({
- targetIconLibary: '@test/aa',
- iconMap: {
- Loading: Star,
- },
- }),
+ replaceIcons({
+ // 目标图标库
+ targetIconLibary: '@test/aa',
+ // 图标映射关系,key为原图标名,value为目标图标名
+ iconMap: {
+ Loading: 'Star',
+ }
+ })
],
11-14
: 配置代码格式需要优化
Taro配置示例使用了HTML代码块标记,但实际上是JS代码。同时配置对象的格式也需要优化。
建议修改为:
-```html
+```js
-{ h5: { compile: { include: [path.resolve(__dirname, '../node_modules')], } },
-mini: { compile: { include: [path.resolve(__dirname, '../node_modules')], } } }
+module.exports = {
+ h5: {
+ compile: {
+ include: [path.resolve(__dirname, '../node_modules')]
+ }
+ },
+ mini: {
+ compile: {
+ include: [path.resolve(__dirname, '../node_modules')]
+ }
+ }
+}
1-4
: 建议补充更多功能说明
功能说明部分过于简单,建议补充:
- 为什么需要替换图标库
- 支持的场景和限制
- 性能影响说明
packages/nutui-replace-icons/src/replace-icons.ts (1)
1-1
: 建议使用类型导入语法
建议使用 import type
语法来明确表示这是类型导入:
-import { IOptions } from './type'
+import type { IOptions } from './type'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
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 (9)
- packages/nutui-replace-icons/.gitignore (1 hunks)
- packages/nutui-replace-icons/README.md (1 hunks)
- packages/nutui-replace-icons/package.json (1 hunks)
- packages/nutui-replace-icons/rollup.config.js (1 hunks)
- packages/nutui-replace-icons/src/index.ts (1 hunks)
- packages/nutui-replace-icons/src/replace-icons.ts (1 hunks)
- packages/nutui-replace-icons/src/type.ts (1 hunks)
- packages/nutui-replace-icons/test/replace-case.test.ts (1 hunks)
- packages/nutui-replace-icons/tsconfig.json (1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/nutui-replace-icons/.gitignore
- packages/nutui-replace-icons/package.json
- packages/nutui-replace-icons/rollup.config.js
- packages/nutui-replace-icons/src/index.ts
- packages/nutui-replace-icons/tsconfig.json
🧰 Additional context used
🪛 Biome
packages/nutui-replace-icons/src/replace-icons.ts
[error] 15-16: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
packages/nutui-replace-icons/test/replace-case.test.ts (1)
1-3
: 导入声明看起来不错!
导入语句简洁明了,包含了所有必要的依赖。
sourceLibrary: string[] | ||
targetLibrary: 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
考虑支持多个目标图标库
目前 targetLibrary
只支持单个目标库,这可能会限制插件的灵活性。建议考虑将其改为数组类型,以支持多个目标库的场景。
建议修改如下:
export interface IOptions {
sourceLibrary: string[]
- targetLibrary: string
+ targetLibrary: string[]
iconMappings?: { [key: string]: 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.
sourceLibrary: string[] | |
targetLibrary: string | |
sourceLibrary: string[] | |
targetLibrary: string[] |
const plugin = replaceIcons({ | ||
targetLibrary: '@test/aa', | ||
iconMappings: { | ||
Loading: 'Star', | ||
}, | ||
}) |
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
建议添加类型定义以增强类型安全性
建议为配置对象添加接口定义,可以防止配置错误并提供更好的开发体验。
+interface IconConfig {
+ targetLibrary: string;
+ iconMappings: Record<string, string>;
+}
-const plugin = replaceIcons({
+const plugin = replaceIcons<IconConfig>({
targetLibrary: '@test/aa',
iconMappings: {
Loading: 'Star',
},
})
📝 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 plugin = replaceIcons({ | |
targetLibrary: '@test/aa', | |
iconMappings: { | |
Loading: 'Star', | |
}, | |
}) | |
interface IconConfig { | |
targetLibrary: string; | |
iconMappings: Record<string, string>; | |
} | |
const plugin = replaceIcons<IconConfig>({ | |
targetLibrary: '@test/aa', | |
iconMappings: { | |
Loading: 'Star', | |
}, | |
}) |
const caseIns = ` | ||
import { Loading } from '@nutui/icons-react' | ||
import { ArrowSize6 as Arrow } from '@nutui/icons-react' | ||
const ReplaceOne = () => { | ||
return <><Loading /> <Arrow /></> | ||
} | ||
` |
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
建议改进测试用例的可维护性
建议将测试用例代码提取为单独的常量或文件,这样可以更好地管理多个测试用例。
+const TEST_CASES = {
+ basicReplacement: `
import { Loading } from '@nutui/icons-react'
import { ArrowSize6 as Arrow } from '@nutui/icons-react'
const ReplaceOne = () => {
return <><Loading /> <Arrow /></>
}
+ `,
+};
-const caseIns = \`...\`
+const caseIns = TEST_CASES.basicReplacement;
📝 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 caseIns = ` | |
import { Loading } from '@nutui/icons-react' | |
import { ArrowSize6 as Arrow } from '@nutui/icons-react' | |
const ReplaceOne = () => { | |
return <><Loading /> <Arrow /></> | |
} | |
` | |
const TEST_CASES = { | |
basicReplacement: ` | |
import { Loading } from '@nutui/icons-react' | |
import { ArrowSize6 as Arrow } from '@nutui/icons-react' | |
const ReplaceOne = () => { | |
return <><Loading /> <Arrow /></> | |
} | |
`, | |
}; | |
const caseIns = TEST_CASES.basicReplacement; |
describe('', () => { | ||
it('replace Loading icons with Star', () => { | ||
const ast = babel.transformSync(caseIns, babelOptions) | ||
// @ts-ignore | ||
expect(ast.code).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.
测试实现需要多处改进
- describe 块缺少描述
- 应避免使用 @ts-ignore
- 建议补充具体的断言来增强测试可靠性
建议按如下方式修改:
-describe('', () => {
+describe('replaceIcons Plugin', () => {
it('replace Loading icons with Star', () => {
const ast = babel.transformSync(caseIns, babelOptions)
- // @ts-ignore
- expect(ast.code).toMatchSnapshot()
+ const result = ast?.code ?? '';
+ expect(result).toMatchSnapshot()
+ // 添加具体断言
+ expect(result).toContain('@test/aa')
+ expect(result).toContain('Star')
+ expect(result).not.toContain('Loading')
})
})
📝 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.
describe('', () => { | |
it('replace Loading icons with Star', () => { | |
const ast = babel.transformSync(caseIns, babelOptions) | |
// @ts-ignore | |
expect(ast.code).toMatchSnapshot() | |
}) | |
}) | |
describe('replaceIcons Plugin', () => { | |
it('replace Loading icons with Star', () => { | |
const ast = babel.transformSync(caseIns, babelOptions) | |
const result = ast?.code ?? ''; | |
expect(result).toMatchSnapshot() | |
// 添加具体断言 | |
expect(result).toContain('@test/aa') | |
expect(result).toContain('Star') | |
expect(result).not.toContain('Loading') | |
}) | |
}) |
### H5 生态下 | ||
|
||
1. 需要将 nutui 组件库包含在编译中。 |
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.
H5环境配置说明不完整
H5环境的配置说明过于简单,缺少具体的配置步骤和示例代码。建议补充:
- babel配置示例
- webpack/vite配置示例
- 完整的使用步骤
需要我帮您补充H5环境的详细配置说明吗?
2. 在 babel.config.js 文件中增加如下代码 | ||
|
||
```js | ||
const { repleaceIcons } = require('@dongdesign/replace-icons') |
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.
包名与函数名存在错误
- 包名与PR标题不一致:使用了
@dongdesign/replace-icons
而不是@nutui/replace-icons
- 函数名拼写错误:
repleaceIcons
应该是replaceIcons
建议修改为:
-const { repleaceIcons } = require('@dongdesign/replace-icons')
+const { replaceIcons } = require('@nutui/replace-icons')
📝 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 { repleaceIcons } = require('@dongdesign/replace-icons') | |
const { replaceIcons } = require('@nutui/replace-icons') |
export function replaceIcons( | ||
options: IOptions = { | ||
sourceLibrary: ['@nutui/icons-react', '@nutui/icons-react-taro'], | ||
targetLibrary: '', | ||
} | ||
) { | ||
if (!options.targetLibrary) { | ||
return {} | ||
} | ||
return replace(options) | ||
} |
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.
建议增强输入验证和错误处理
当前实现在参数验证和错误处理方面较为简单,建议增加以下功能:
- 验证 sourceLibrary 是否为非空数组
- 验证 targetLibrary 的格式
- 验证 iconMappings 的结构
建议修改如下:
export function replaceIcons(
options: IOptions = {
sourceLibrary: ['@nutui/icons-react', '@nutui/icons-react-taro'],
targetLibrary: '',
}
) {
+ // Validate sourceLibrary
+ if (!Array.isArray(options.sourceLibrary) || options.sourceLibrary.length === 0) {
+ throw new Error('sourceLibrary must be a non-empty array')
+ }
+
+ // Validate targetLibrary
if (!options.targetLibrary) {
- return {}
+ throw new Error('targetLibrary is required')
}
+
+ // Validate iconMappings if provided
+ if (options.iconMappings && typeof options.iconMappings !== 'object') {
+ throw new Error('iconMappings must be an object')
+ }
+
return replace(options)
}
📝 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.
export function replaceIcons( | |
options: IOptions = { | |
sourceLibrary: ['@nutui/icons-react', '@nutui/icons-react-taro'], | |
targetLibrary: '', | |
} | |
) { | |
if (!options.targetLibrary) { | |
return {} | |
} | |
return replace(options) | |
} | |
export function replaceIcons( | |
options: IOptions = { | |
sourceLibrary: ['@nutui/icons-react', '@nutui/icons-react-taro'], | |
targetLibrary: '', | |
} | |
) { | |
// Validate sourceLibrary | |
if (!Array.isArray(options.sourceLibrary) || options.sourceLibrary.length === 0) { | |
throw new Error('sourceLibrary must be a non-empty array') | |
} | |
// Validate targetLibrary | |
if (!options.targetLibrary) { | |
throw new Error('targetLibrary is required') | |
} | |
// Validate iconMappings if provided | |
if (options.iconMappings && typeof options.iconMappings !== 'object') { | |
throw new Error('iconMappings must be an object') | |
} | |
return replace(options) | |
} |
function replace(options: IOptions) { | ||
const sourceLibrary = options.sourceLibrary || [] | ||
const targetLibrary = options.targetLibrary | ||
return { | ||
visitor: { | ||
ImportDeclaration(path) { | ||
if (sourceLibrary.indexOf(path.node.source.value) > -1) { | ||
// 替换包名称 | ||
path.node.source.value = targetLibrary | ||
path.node.specifiers.forEach((specifier) => { | ||
// 根据 iconMappings 进行替换 | ||
const iconMappings = options.iconMappings | ||
if (iconMappings && iconMappings[specifier.imported.name]) { | ||
specifier.imported.name = iconMappings[specifier.imported.name] | ||
} | ||
}) | ||
} | ||
}, | ||
}, | ||
} | ||
} |
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
代码可以进行以下优化
当前实现有以下几点可以改进:
- 使用更现代的数组方法
- 增加错误处理
- 使用可选链
- 添加英文注释以保持一致性
建议按照以下方式重构:
function replace(options: IOptions) {
const sourceLibrary = options.sourceLibrary || []
const targetLibrary = options.targetLibrary
return {
visitor: {
ImportDeclaration(path) {
- if (sourceLibrary.indexOf(path.node.source.value) > -1) {
+ if (sourceLibrary.includes(path.node.source.value)) {
- // 替换包名称
+ // Replace package name
path.node.source.value = targetLibrary
path.node.specifiers.forEach((specifier) => {
- // 根据 iconMappings 进行替换
+ // Replace based on iconMappings
const iconMappings = options.iconMappings
- if (iconMappings && iconMappings[specifier.imported.name]) {
+ if (iconMappings?.[specifier.imported.name]) {
specifier.imported.name = iconMappings[specifier.imported.name]
+ } else {
+ console.warn(`No mapping found for icon: ${specifier.imported.name}`)
}
})
}
},
},
}
}
📝 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.
function replace(options: IOptions) { | |
const sourceLibrary = options.sourceLibrary || [] | |
const targetLibrary = options.targetLibrary | |
return { | |
visitor: { | |
ImportDeclaration(path) { | |
if (sourceLibrary.indexOf(path.node.source.value) > -1) { | |
// 替换包名称 | |
path.node.source.value = targetLibrary | |
path.node.specifiers.forEach((specifier) => { | |
// 根据 iconMappings 进行替换 | |
const iconMappings = options.iconMappings | |
if (iconMappings && iconMappings[specifier.imported.name]) { | |
specifier.imported.name = iconMappings[specifier.imported.name] | |
} | |
}) | |
} | |
}, | |
}, | |
} | |
} | |
function replace(options: IOptions) { | |
const sourceLibrary = options.sourceLibrary || [] | |
const targetLibrary = options.targetLibrary | |
return { | |
visitor: { | |
ImportDeclaration(path) { | |
if (sourceLibrary.includes(path.node.source.value)) { | |
// Replace package name | |
path.node.source.value = targetLibrary | |
path.node.specifiers.forEach((specifier) => { | |
// Replace based on iconMappings | |
const iconMappings = options.iconMappings | |
if (iconMappings?.[specifier.imported.name]) { | |
specifier.imported.name = iconMappings[specifier.imported.name] | |
} else { | |
console.warn(`No mapping found for icon: ${specifier.imported.name}`) | |
} | |
}) | |
} | |
}, | |
}, | |
} | |
} |
🧰 Tools
🪛 Biome
[error] 15-16: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
@nutui/replace-icons
功能说明
用于替换组件库内置的 Icon 库。
使用方法
原始代码
替换后代码
Summary by CodeRabbit
新功能
replaceIcons
函数,作为替换图标的主要入口点。IOptions
接口,定义替换图标所需的选项结构。文档
README.md
,提供详细的使用说明和代码示例。测试
replace-icons
功能的新测试文件,验证图标替换逻辑的正确性。版本更新
@nutui/replace-icons
包版本更新至1.0.1
。