-
Notifications
You must be signed in to change notification settings - Fork 258
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: auto import css plugin #2613
base: dev-harmony
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2613 +/- ##
=======================================
Coverage 82.95% 82.95%
=======================================
Files 219 219
Lines 17908 17908
Branches 2547 2547
=======================================
Hits 14856 14856
Misses 3047 3047
Partials 5 5 ☔ View full report in Codecov by Sentry. |
a2de117
to
a351e20
Compare
Walkthrough此次更改涉及在 Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 (
|
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: 12
🧹 Outside diff range and nitpick comments (9)
packages/nutui-auto-import/src/rollup.ts (1)
3-3
: 默认导出看起来不错,但可以考虑增加一些注释默认导出使用类型断言确保了类型安全,这是一个很好的做法。为了提高代码的可读性和可维护性,您可以考虑添加一个简短的注释来解释这个导出的用途。
建议添加注释:
+ // 导出 NutUIAutoImport 的 rollup 方法作为默认导出 export default NutUIAutoImport.rollup as typeof NutUIAutoImport.rollup
packages/nutui-auto-import/tests/fixtures/basic.jsx (1)
1-1
: 移除未使用的导入当前代码中导入了
Button
组件,但在文件中并未使用。建议移除未使用的导入以优化代码。建议应用以下更改:
-import { Button, Cell } from '@nutui/nutui-react' +import { Cell } from '@nutui/nutui-react'packages/nutui-auto-import/tsup.config.ts (1)
1-11
: 配置文件看起来结构良好,但可以考虑一些改进这个 tsup 配置文件整体结构清晰,涵盖了主要的构建选项。以下是一些观察和建议:
- 入口点配置使用了通配符,这可能会包含不需要的文件。考虑明确指定入口文件。
- 目标环境设置为 Node.js 18.12,确保这与项目的最低支持版本一致。
- 没有看到
minify
选项,如果需要最小化输出,可以添加此选项。- 可以考虑添加
sourcemap
选项,以便于调试。建议考虑以下改进:
import { defineConfig } from 'tsup' export default defineConfig({ - entry: ['./src/*.ts'], + entry: ['./src/index.ts'], // 或其他明确的入口文件 format: ['cjs', 'esm'], target: 'node18.12', splitting: true, cjsInterop: true, clean: true, dts: true, + minify: true, + sourcemap: true, })这些更改将使配置更加明确和全面。
packages/nutui-auto-import/tests/rollup.test.ts (3)
1-5
: 导入看起来很好,但建议添加注释说明 @sxzz/test-utils 的用途。导入的模块适合设置 Rollup 测试环境。使用
node:path
确保了与较新版本 Node.js 的兼容性。然而,@sxzz/test-utils
不是一个广泛使用的库。建议添加一个简短的注释,解释为什么选择使用
@sxzz/test-utils
,以及它在这个测试套件中的具体作用。这将有助于其他开发者理解代码的意图和依赖关系。
7-9
: 测试套件结构良好,但可以添加更多注释以提高可读性。测试套件的结构使用了 Vitest 的
describe
函数和异步函数,这对于处理异步测试是正确的。testFixtures
函数的使用看起来也是正确的。建议添加以下改进:
- 为
testFixtures
函数添加注释,解释其功能和参数的作用。- 解释为什么使用
import.meta.dirname
而不是__dirname
。- 考虑添加一个简短的注释,说明这个测试套件的整体目的。
这些添加将有助于其他开发者更快地理解测试的结构和目的。
Also applies to: 22-24
10-21
: Rollup 构建配置看起来不错,但可以考虑一些改进。Rollup 构建配置适合测试 NutUIAutoImport 插件。使用
snapshot
进行输出比较是一个好方法。考虑以下建议来改进代码:
- 将 NutUIAutoImport 的配置选项提取为一个常量对象,这样可以在多个测试中重用。
const nutUIAutoImportOptions = { libraryName: '@nutui/nutui-react', style: 'css', taro: false, }; // 然后在测试中使用 NutUIAutoImport(nutUIAutoImportOptions),
添加注释解释为什么
taro
选项被设置为false
。考虑添加更多的测试用例,覆盖不同的 NutUIAutoImport 配置选项。
这些改进将增加代码的可维护性和测试覆盖率。
packages/nutui-auto-import/package.json (1)
79-88
: 脚本配置全面,建议添加文档生成脚本scripts 字段包含了开发所需的所有常见任务,如 lint、build、test 等。使用 bumpp 进行版本管理是个好做法。prepublishOnly 脚本确保了发布前会重新构建包,这很重要。
建议:考虑添加一个生成文档的脚本,例如:
"docs": "typedoc --out docs src"这将有助于保持文档的最新状态。
packages/nutui-auto-import/src/core/options.ts (1)
3-9
: 建议为Options
接口添加注释为了提高代码的可读性和可维护性,建议为
Options
接口的每个属性添加 JSDoc 注释,帮助其他开发者理解各个选项的作用。packages/nutui-auto-import/src/index.ts (1)
36-54
: 未使用的变量components
在第 36 行声明了变量
components
,但在后续代码中未使用。建议删除未使用的变量,以保持代码简洁。可以删除未使用的变量:
- const components = []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
packages/nutui-auto-import/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
packages/nutui-auto-import/tests/__snapshots__/rollup.test.ts.snap
is excluded by!**/*.snap
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
- packages/nutui-auto-import/.editorconfig (1 hunks)
- packages/nutui-auto-import/.gitattributes (1 hunks)
- packages/nutui-auto-import/.gitignore (1 hunks)
- packages/nutui-auto-import/LICENSE (1 hunks)
- packages/nutui-auto-import/README.md (1 hunks)
- packages/nutui-auto-import/babel.config.json (1 hunks)
- packages/nutui-auto-import/package.json (1 hunks)
- packages/nutui-auto-import/src/api.ts (1 hunks)
- packages/nutui-auto-import/src/core/options.ts (1 hunks)
- packages/nutui-auto-import/src/esbuild.ts (1 hunks)
- packages/nutui-auto-import/src/index.ts (1 hunks)
- packages/nutui-auto-import/src/rolldown.ts (1 hunks)
- packages/nutui-auto-import/src/rollup.ts (1 hunks)
- packages/nutui-auto-import/src/rspack.ts (1 hunks)
- packages/nutui-auto-import/src/vite.ts (1 hunks)
- packages/nutui-auto-import/src/webpack.ts (1 hunks)
- packages/nutui-auto-import/tests/fixtures/basic.jsx (1 hunks)
- packages/nutui-auto-import/tests/rollup.test.ts (1 hunks)
- packages/nutui-auto-import/tsconfig.json (1 hunks)
- packages/nutui-auto-import/tsup.config.ts (1 hunks)
- packages/nutui-auto-import/vitest.config.ts (1 hunks)
- pnpm-workspace.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (14)
- packages/nutui-auto-import/.editorconfig
- packages/nutui-auto-import/.gitattributes
- packages/nutui-auto-import/.gitignore
- packages/nutui-auto-import/LICENSE
- packages/nutui-auto-import/README.md
- packages/nutui-auto-import/babel.config.json
- packages/nutui-auto-import/src/api.ts
- packages/nutui-auto-import/src/esbuild.ts
- packages/nutui-auto-import/src/rolldown.ts
- packages/nutui-auto-import/src/rspack.ts
- packages/nutui-auto-import/src/vite.ts
- packages/nutui-auto-import/src/webpack.ts
- packages/nutui-auto-import/tsconfig.json
- packages/nutui-auto-import/vitest.config.ts
🧰 Additional context used
🪛 yamllint
pnpm-workspace.yaml
[error] 4-4: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
packages/nutui-auto-import/src/rollup.ts (1)
1-1
: 导入语句看起来不错!导入语句简洁明了,符合最佳实践。
packages/nutui-auto-import/package.json (4)
27-29
: 主入口点配置正确main、module 和 types 字段的配置正确,支持 CommonJS 和 ES 模块,并提供了 TypeScript 类型定义。这是现代 JavaScript 包的良好实践。
30-75
: 导出配置全面且灵活exports 字段的配置非常全面,为不同的构建系统(如 Vite、Webpack、Rollup 等)提供了专门的入口点。同时支持 CommonJS 和 ES 模块格式,这种配置方式非常灵活,能够满足各种项目的需求。catch-all 导出确保了所有文件都可以被访问。这是一个优秀的配置示例。
76-78
: 发布配置正确publishConfig 设置为 public access 是正确的,这确保了包可以作为公共 npm 包发布。
115-117
: Node.js 版本要求合理将最低 Node.js 版本要求设置为 18.12.0 是合理的。这个版本是一个较新的 LTS 版本,可以确保访问现代 JavaScript 特性和improved性能。这个选择在保持兼容性和利用新特性之间取得了很好的平衡。
packages/nutui-auto-import/src/index.ts (1)
32-34
: 确认this.parse
方法的可用性在第 34 行,使用了
this.parse(code)
方法,并添加了// @ts-ignore
注释。请确保在当前插件环境中,this.parse
方法是可用的。如果不可用,可能需要使用其他解析方式,如直接使用@babel/parser
。
export const A = () => { | ||
return <Cell>Test</Cell> | ||
} |
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
改进组件命名和类型定义
当前导出的组件命名为 A
,这个名称不够描述性,也不符合 React 组件的命名约定。此外,组件缺少 PropTypes 或 TypeScript 类型定义。
建议进行以下改进:
- 使用更具描述性的名称,例如
TestCell
。 - 添加 PropTypes 或 TypeScript 类型定义。
示例改进:
import React from 'react';
import { Cell } from '@nutui/nutui-react';
import PropTypes from 'prop-types';
export const TestCell = ({ text }) => {
return <Cell>{text}</Cell>;
};
TestCell.propTypes = {
text: PropTypes.string.isRequired,
};
// 或者使用 TypeScript:
// interface TestCellProps {
// text: string;
// }
//
// export const TestCell: React.FC<TestCellProps> = ({ text }) => {
// return <Cell>{text}</Cell>;
// };
这样的改进可以提高代码的可读性和可维护性。
// console.log(Button) | ||
console.log(Cell) |
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.
移除控制台日志语句
在生产环境中,应该移除所有的 console.log
语句。这些语句通常用于调试,但不应该出现在最终的代码中。
建议删除这两行:
-// console.log(Button)
-console.log(Cell)
如果这些日志对于开发过程很重要,可以考虑使用条件性日志记录或专门的日志库。
📝 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.
// console.log(Button) | |
console.log(Cell) |
"name": "@nutui/nutui-react-auto-import", | ||
"version": "0.0.1", | ||
"packageManager": "[email protected]", | ||
"description": "Description.", |
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.
请更新包的描述
当前的描述 "Description." 过于简单,不能充分说明包的用途。建议提供一个更具体、信息丰富的描述,以帮助用户理解这个包的功能和用途。
例如:
"NutUI React 组件的自动导入插件,支持多种构建工具如 Vite、Webpack、Rollup 等。"
"dependencies": { | ||
"@nutui/nutui-react": "^2.6.22", | ||
"@rollup/pluginutils": "^5.1.1", | ||
"@types/babel__traverse": "^7.20.6", | ||
"estree-walker": "^3.0.3", | ||
"magic-string": "^0.30.11", | ||
"unplugin": "^1.14.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.
建议为依赖项指定版本范围
当前的依赖项配置没有指定版本范围,这可能会在未来的更新中导致兼容性问题。建议使用语义化版本范围来限制依赖项的版本。例如:
"dependencies": {
"@nutui/nutui-react": "^2.6.22",
"@rollup/pluginutils": "^5.1.1",
"@types/babel__traverse": "^7.20.6",
"estree-walker": "^3.0.3",
"magic-string": "^0.30.11",
"unplugin": "^1.14.1"
}
这样可以确保获取补丁和小版本更新,同时避免可能破坏性的大版本更新。
"devDependencies": { | ||
"@babel/core": "^7.25.8", | ||
"@babel/preset-react": "^7.25.7", | ||
"@rollup/plugin-babel": "^6.0.4", | ||
"@sxzz/eslint-config": "^4.2.0", | ||
"@sxzz/prettier-config": "^2.0.2", | ||
"@sxzz/test-utils": "^0.3.2", | ||
"@types/node": "^22.5.5", | ||
"bumpp": "^9.5.2", | ||
"eslint": "^9.11.0", | ||
"fast-glob": "^3.3.2", | ||
"prettier": "^3.3.3", | ||
"tsup": "^8.3.0", | ||
"tsx": "^4.19.1", | ||
"typescript": "^5.6.2", | ||
"vite": "^5.4.7", | ||
"vitest": "^2.1.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.
建议为开发依赖指定版本范围并使用稳定版本
与生产依赖类似,开发依赖也应该指定版本范围。此外,一些包(如 eslint 和 @types/node)使用了非常新的版本,可能会导致不稳定性。建议:
- 为所有开发依赖指定版本范围。
- 考虑使用更稳定的版本。例如:
"devDependencies": {
"@babel/core": "^7.22.0",
"@babel/preset-react": "^7.22.0",
"@rollup/plugin-babel": "^6.0.0",
"@sxzz/eslint-config": "^4.0.0",
"@sxzz/prettier-config": "^2.0.0",
"@sxzz/test-utils": "^0.3.0",
"@types/node": "^18.0.0",
"bumpp": "^9.0.0",
"eslint": "^8.0.0",
"fast-glob": "^3.0.0",
"prettier": "^3.0.0",
"tsup": "^8.0.0",
"tsx": "^4.0.0",
"typescript": "^5.0.0",
"vite": "^5.0.0",
"vitest": "^2.0.0"
}
这将有助于提高项目的稳定性和可预测性。
export function getLibraryName(options: Options): string { | ||
if (options.libraryName) { | ||
return options.libraryName | ||
} | ||
return '@nutui/nutui-react' | ||
} |
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
优化 getLibraryName
函数的实现
可以使用更简洁的语法来实现相同的功能,提高代码的可读性。
建议修改为:
export function getLibraryName(options: Options): string {
return options.libraryName || '@nutui/nutui-react'
}
const name = specifier.imported.name.toLowerCase() | ||
if (absolutePath.length === 0 || !absolutePath[0]) return |
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.
组件名大小写处理可能导致问题
在第 56 行,将导入的组件名转换为小写:
const name = specifier.imported.name.toLowerCase()
如果组件名是大写字母开头(例如 Button
),而实际文件夹名称也是大写,这种转换可能导致路径错误。
建议移除 toLowerCase()
,以保留正确的组件名:
- const name = specifier.imported.name.toLowerCase()
+ const name = specifier.imported.name
magicString.prepend( | ||
`import "${absolutePath[0]}${libraryName}/dist/es/packages/${name}/style/css.js";` | ||
) | ||
} else if (options.style) { | ||
magicString.prepend( | ||
`import "${absolutePath[0]}${libraryName}/dist/es/packages/${name}/style/index.js";` | ||
) | ||
} | ||
} |
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.
路径拼接的鲁棒性不足
在第 59-65 行,直接使用了 absolutePath[0]
来拼接路径。如果 require.resolve
失败,absolutePath[0]
可能未定义,导致路径拼接出错。还需考虑操作系统的路径分隔符差异(如 Windows 的 \
和 Unix 的 /
)。
建议在使用 absolutePath[0]
前,检查其有效性,并使用 path
模块来处理路径拼接:
import path from 'path'
// 在使用前检查 absolutePath[0]
if (absolutePath[0]) {
const stylePath = path.join(
absolutePath[0],
libraryName,
'dist',
'es',
'packages',
name,
'style',
options.style === 'css' ? 'css.js' : 'index.js'
)
magicString.prepend(`import "${stylePath}";`)
}
enter(node) { | ||
if (node.type === 'ImportDeclaration') { | ||
// console.log(node) | ||
if (node.source.value !== libraryName) return | ||
|
||
let absolutePath: string[] = [] | ||
try { | ||
const resolvePath = require.resolve(libraryName) | ||
absolutePath = resolvePath.split(libraryName) | ||
} catch (e) { | ||
/* empty */ | ||
console.log(`warn: cannot reslove ${libraryName}`) | ||
} | ||
node.specifiers.forEach((specifier) => { | ||
if (specifier.type === 'ImportSpecifier') { | ||
// 把节点存下来 | ||
components.push(specifier) | ||
// @ts-ignore | ||
const name = specifier.imported.name.toLowerCase() | ||
if (absolutePath.length === 0 || !absolutePath[0]) return | ||
if (options.style === 'css') { | ||
magicString.prepend( | ||
`import "${absolutePath[0]}${libraryName}/dist/es/packages/${name}/style/css.js";` | ||
) | ||
} else if (options.style) { | ||
magicString.prepend( | ||
`import "${absolutePath[0]}${libraryName}/dist/es/packages/${name}/style/index.js";` | ||
) | ||
} | ||
} | ||
}) | ||
} | ||
}, |
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
缺少对其他导入类型的支持
当前仅处理了命名导入 (ImportSpecifier
),未考虑默认导入 (ImportDefaultSpecifier
) 和命名空间导入 (ImportNamespaceSpecifier
)。这可能导致当用户以不同方式导入库时,插件无法正确处理。
建议增加对其他导入类型的处理:
node.specifiers.forEach((specifier) => {
if (
specifier.type === 'ImportSpecifier' ||
specifier.type === 'ImportDefaultSpecifier' ||
specifier.type === 'ImportNamespaceSpecifier'
) {
// 处理不同的导入类型
const name = specifier.local.name
// 后续代码...
}
})
} catch (e) { | ||
/* empty */ | ||
console.log(`warn: cannot reslove ${libraryName}`) | ||
} |
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.
拼写错误和注释不准确
在 catch
块中,注释 /* empty */
与实际情况不符,因为您在其中记录了警告信息。此外,console.log
中的 reslove
应为 resolve
。
请应用以下修改来修正拼写错误并更新注释:
} catch (e) {
- /* empty */
- console.log(`warn: cannot reslove ${libraryName}`)
+ // 警告:无法解析库名
+ console.log(`warn: cannot resolve ${libraryName}`)
}
📝 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.
} catch (e) { | |
/* empty */ | |
console.log(`warn: cannot reslove ${libraryName}`) | |
} | |
} catch (e) { | |
// 警告:无法解析库名 | |
console.log(`warn: cannot resolve ${libraryName}`) | |
} |
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
文档
测试