feat: support unplugin context (again) #1741
Conversation
* feat: support plugin context * fix: napi context * chore: revert changes * chore: improve * feat: add error * feat: warn and error support object * feat: support emit_file * ci: fix test * chore: improve * chore: update test * chore: format * chore: don't support add watch file * feat: load and transform adapter, and add unplugin-replace example * chore: test unplugin-icons * chore: update pnpm-lock.yaml * docs: improve --------- Co-authored-by: xusd320 <[email protected]>
概述遍历这个拉取请求引入了对 Mako 构建系统的重大增强，主要集中在插件系统的上下文管理。通过引入 变更
序列图sequenceDiagram
participant Plugin
participant PluginContext
participant BuildSystem
Plugin->>PluginContext: 创建上下文实例
Plugin->>BuildSystem: 调用钩子函数
BuildSystem->>PluginContext: 传递上下文
PluginContext-->>Plugin: 提供 warn/error/emitFile 方法
可能相关的 PR
诗歌
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
packages/mako/src/index.ts (2)
221-244: 改进 'emitFile' 方法的临时文件处理在
emitFile方法中，当file.type为'asset'时，通过生成临时文件存储内容并再进行 emit。这种方式可能存在文件名冲突的风险。建议使用fs.mkdtempSync创建唯一的临时目录，确保临时文件的安全性和唯一性。
335-348: 完善 'adapterResult' 函数的返回值处理在
adapterResult函数中，建议处理更多类型的返回值。例如，当result为null或undefined时，明确处理方式，以提高函数的健壮性。e2e/fixtures/plugins.unplugin/expect.js (1)
7-8: 测试断言完善，建议优化错误信息测试用例很好地覆盖了两个插件的功能验证。建议为断言添加更详细的错误信息，包含实际值和期望值，以便调试失败案例。
-assert(content.includes('fooooooo'), `should replace FOOOO with "fooooooo" with unplugin-replace`); +assert(content.includes('fooooooo'), `应该使用 unplugin-replace 将 FOOOO 替换为 "fooooooo"。\n实际内容：${content}`); -assert(content.includes('fill: "currentColor",'), `should include fill: "currentColor" with unplugin-icons`); +assert(content.includes('fill: "currentColor",'), `使用 unplugin-icons 后应该包含 fill: "currentColor"。\n实际内容：${content}`);docs/config.zh-CN.md (3)
599-605: 完善插件上下文方法的文档说明建议对新增的插件上下文方法做如下改进：
- 为每个方法添加更详细的使用示例
- 修正语法错误：
添加一个错误应改为添加一个异常建议按以下方式修改：
你还可以在 hook 函数里用以下方法。 - - `this.emitFile({ type: 'asset', fileName: string, source: string | Uint8Array })`, 添加文件到输出目录 - - `this.warn(message: string)`, 添加一个警告 - - `this.error(message: string)`, 添加一个错误 + - `this.emitFile({ type: 'asset', fileName: string, source: string | Uint8Array })`, 添加文件到输出目录。例如：添加静态资源文件。 + - `this.warn(message: string)`, 添加一个警告。例如：提示配置项即将废弃。 + - `this.error(message: string)`, 添加一个异常。例如：配置项验证失败。
604-605: 标注未实现的功能对于尚未支持的功能，建议添加更明确的说明，以避免用户误用。
建议按以下方式修改：
- - `this.parse(code: string)`, 解析代码 (CURRENTLY NOT SUPPORTED) - - `this.addWatchFile(filePath: string)`, 添加一个监听文件 (CURRENTLY NOT SUPPORTED) + - `this.parse(code: string)`, 解析代码 (🚧 计划中，暂未实现) + - `this.addWatchFile(filePath: string)`, 添加一个监听文件 (🚧 计划中，暂未实现)
607-607: 补充 unplugin 兼容性说明建议添加更多关于 unplugin 兼容性的具体信息和使用示例。
建议按以下方式补充文档：
Plugins 兼容 [unplugin](https://unplugin.unjs.io/)，所以你可以使用 unplugin 的插件，比如 [unplugin-icons](https://github.com/unplugin/unplugin-icons), [unplugin-replace](https://github.com/unplugin/unplugin-replace) 等。 + + 使用示例： + + ```js + // mako.config.js + import Icons from 'unplugin-icons/mako' + import Replace from 'unplugin-replace/mako' + + export default { + plugins: [ + Icons({ + compiler: 'jsx', + jsx: 'react' + }), + Replace({ + values: { + __VERSION__: '1.0.0' + } + }) + ] + } + ```docs/config.md (1)
601-607: 修正英文文档中的语法错误文档中存在以下语法问题需要修正：
- 使用不当的冠词
- 标点符号使用不规范
- 动词单复数不一致
建议按以下方式修改：
And you can also use this methods in hook functions. - `this.emitFile({ type: 'asset', fileName: string, source: string | Uint8Array })`, emit a file - `this.warn(message: string)`, emit a warning -- `this.error(message: string)`, emit a error +- `this.error(message: string)`, emit an error - `this.parse(code: string)`, parse the code (CURRENTLY NOT SUPPORTED) - `this.addWatchFile(filePath: string)`, add a watch file (CURRENTLY NOT SUPPORTED) -Plugins is compatible with [unplugin] +Plugins are compatible with [unplugin]🧰 Tools
🪛 LanguageTool
[uncategorized] ~603-~603: Loose punctuation mark.
Context: ... string, source: string | Uint8Array }), emit a file -this.warn(message: strin...(UNLIKELY_OPENING_PUNCTUATION)
[misspelling] ~605-~605: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...g -this.error(message: string), emit a error -this.parse(code: string), par...(EN_A_VS_AN)
e2e/fixtures/plugins.context/plugins.config.js (1)
8-24: 建议改进错误处理机制当前实现在遇到
.hoo文件时同时发出警告和错误，这可能会导致混淆。建议：
- 分离错误和警告的测试场景
- 添加更多的错误处理边界情况
packages/mako/binding.d.ts (1)
275-279: 建议增强类型定义
PluginContext类的方法定义可以更加严谨：
msg参数应该允许接受对象类型emitFile方法的参数可以添加可选的配置选项建议修改为：
export class PluginContext { - warn(msg: string): void; + warn(msg: string | Record<string, unknown>): void; error(msg: string): void; - emitFile(originPath: string, outputPath: string): void; + emitFile(originPath: string, outputPath: string, options?: { overwrite?: boolean }): void; }crates/binding/src/js_plugin.rs (2)
47-51: 优化 emit_file 方法的线程安全性
emit_file方法中的锁定范围可以优化，建议：
- 减少锁定时间
- 使用
parking_lot提供的锁以提高性能- 添加错误处理
71-73: 优化上下文克隆的性能代码中多处使用
context.clone()，这可能导致不必要的性能开销。建议：
- 考虑使用引用计数而不是克隆
- 实现
Clonetrait 的自定义实现- 使用
Cow来避免不必要的克隆Also applies to: 81-86, 90-95, 116-118, 151-155, 168-170, 182-184, 197-199, 216-221, 234-240
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
e2e/fixtures/plugins/node_modules/plugin/index.jsis excluded by!**/node_modules/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
crates/binding/src/js_hook.rs(2 hunks)crates/binding/src/js_plugin.rs(8 hunks)crates/mako/src/plugin.rs(2 hunks)crates/mako/src/plugins/bundless_compiler.rs(1 hunks)docs/config.md(1 hunks)docs/config.zh-CN.md(1 hunks)e2e/fixtures/plugins.context/expect.js(1 hunks)e2e/fixtures/plugins.context/mako.config.json(1 hunks)e2e/fixtures/plugins.context/plugin.js(1 hunks)e2e/fixtures/plugins.context/plugins.config.js(1 hunks)e2e/fixtures/plugins.context/src/foo.hoo(1 hunks)e2e/fixtures/plugins.context/src/index.tsx(1 hunks)e2e/fixtures/plugins.unplugin/expect.js(1 hunks)e2e/fixtures/plugins.unplugin/mako.config.json(1 hunks)e2e/fixtures/plugins.unplugin/plugins.config.js(1 hunks)e2e/fixtures/plugins.unplugin/src/index.tsx(1 hunks)package.json(2 hunks)packages/mako/binding.d.ts(1 hunks)packages/mako/binding.js(1 hunks)packages/mako/src/index.ts(4 hunks)scripts/mako.js(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- e2e/fixtures/plugins.context/src/foo.hoo
- e2e/fixtures/plugins.context/mako.config.json
- e2e/fixtures/plugins.context/src/index.tsx
- e2e/fixtures/plugins.unplugin/mako.config.json
🧰 Additional context used
🪛 LanguageTool
docs/config.md
[uncategorized] ~603-~603: Loose punctuation mark.
Context: ... string, source: string | Uint8Array }), emit a file - this.warn(message: strin...
(UNLIKELY_OPENING_PUNCTUATION)
[misspelling] ~605-~605: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...g - this.error(message: string), emit a error - this.parse(code: string), par...
(EN_A_VS_AN)
[uncategorized] ~609-~609: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct.
Context: ...file (CURRENTLY NOT SUPPORTED) Plugins is compatible with [unplugin](https://unpl...
(AI_HYDRA_LEO_CPT_IS_ARE)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: stable - i686-pc-windows-msvc - node@20
- GitHub Check: stable - x86_64-pc-windows-msvc - node@20
- GitHub Check: stable - aarch64-unknown-linux-musl - node@20
- GitHub Check: stable - aarch64-unknown-linux-gnu - node@20
- GitHub Check: Clippy
- GitHub Check: stable - x86_64-unknown-linux-musl - node@20
- GitHub Check: Mako Build (macos-latest)
- GitHub Check: stable - x86_64-unknown-linux-gnu - node@20
- GitHub Check: Mako Build (ubuntu-latest)
- GitHub Check: Cargo Test
- GitHub Check: stable - aarch64-apple-darwin - node@20
- GitHub Check: Code Coverage
- GitHub Check: stable - x86_64-apple-darwin - node@20
🔇 Additional comments (15)
crates/binding/src/js_hook.rs (2)
6-6: 引入 'PluginContext'引入了 'PluginContext' 模块，以在 hooks 中提供插件上下文支持，增强插件与系统的交互能力。
85-99: 在 'TsFnHooks' 中为 hooks 添加 'PluginContext' 参数在 'TsFnHooks' 结构体中，为各个 hook 添加了 'PluginContext' 参数，使得在执行 hooks 时，可以传递插件上下文，提升了插件的功能和可扩展性。
packages/mako/src/index.ts (3)
2-2: 引入 'os' 模块新增了对 'os' 模块的引入，用于在后续代码中处理与操作系统相关的功能。
Line range hint
145-152: 支持导出 Mako 配置添加了根据环境变量
DUMP_MAKO_CONFIG导出 Mako 配置到mako.config.json的功能，方便调试和配置管理。
196-314: 为每个插件的 hook 添加上下文通过遍历插件，为每个插件的 hook 添加了上下文支持，并防止重复补丁，确保插件不会被多次补丁处理。这样增强了插件的可扩展性和稳定性。
e2e/fixtures/plugins.unplugin/src/index.tsx (2)
4-4: 注意：使用了未定义的变量
FOOOO变量未定义，但这是有意为之，因为它将被 unplugin-replace 插件替换为 "fooooooo"。这是测试插件替换功能的一部分。
5-5: 代码实现正确组件实现简洁明了，正确使用了图标组件。这为测试 unplugin-icons 的功能提供了良好的用例。
e2e/fixtures/plugins.unplugin/plugins.config.js (1)
4-18: 插件配置完善插件配置符合测试需求：
- unplugin-replace 正确配置了字符串替换规则
- unplugin-icons 正确设置了 JSX 和 React 支持
- 禁用 autoInstall 对测试环境来说是合适的选择
e2e/fixtures/plugins.context/expect.js (1)
6-7: 测试用例设计合理测试逻辑清晰，正确验证了构建结果中的文件内容。断言明确且具有针对性。
packages/mako/binding.js (1)
307-309: 导出更改看起来正确新增 PluginContext 的导出符合预期，实现正确。
crates/mako/src/plugin.rs (2)
187-192: 插件接口更新：添加上下文参数在
before_write_fs方法中添加context参数是一个很好的改进，这使得插件可以在文件写入前访问更多的上下文信息。
430-434: 实现更新：传递上下文参数
PluginDriver中的实现正确地将上下文传递给了每个插件的before_write_fs方法。crates/mako/src/plugins/bundless_compiler.rs (1)
133-133: 正确传递上下文参数在调用
before_write_fs时正确地传递了self.context，与插件接口的更新保持一致。scripts/mako.js (2)
10-14: 改进：提取监视模式标志将 watch 模式的判断提取为变量并传递给 build 函数，提高了代码的可读性和可维护性。
16-24: 改进：进程退出处理添加了合适的进程退出处理：
- 非监视模式下成功构建后优雅退出
- 错误情况下正确处理并退出
| async load(path) { | ||
| path.endsWith('.hoo'); | ||
| } |
There was a problem hiding this comment.
未使用 path.endsWith 的结果
在 load 方法中，调用了 path.endsWith('.hoo')，但没有使用其返回值。建议使用返回值以实现预期的逻辑，例如：
if (path.endsWith('.hoo')) {
// 实现相应的加载逻辑
}| @@ -25,6 +25,7 @@ | |||
| "@biomejs/biome": "^1.4.1", | |||
| "@emotion/react": "^11.11.1", | |||
| "@emotion/styled": "^11.11.0", | |||
| "@iconify-json/mdi": "^1.2.2", | |||
There was a problem hiding this comment.
💡 Codebase verification
缺少必要的依赖项
为确保 unplugin-icons 在 React 项目中正常工作，请添加以下依赖：
- @svgr/core (>= 7.0.0)
其他新增的依赖项（@iconify-json/mdi 和 unplugin-replace）兼容性正常，无需额外配置。
🔗 Analysis chain
新增依赖：支持 unplugin
添加了以下开发依赖：
- @iconify-json/mdi：图标资源
- unplugin-icons：图标插件
- unplugin-replace：替换插件
建议：
- 确保这些依赖的版本与项目的其他依赖兼容
- 考虑添加相关的类型声明文件
Also applies to: 59-60
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查依赖兼容性
npm info [email protected] peerDependencies
npm info [email protected] peerDependencies
Length of output: 321
Script:
#!/bin/bash
# Check package.json for framework dependencies and configuration
cat package.json
# Look for Vue or Svelte configuration files
fd -e vue -e svelte -e config.js -e config.ts
Length of output: 2921
Script:
#!/bin/bash
# Check for @svgr/core in dependencies
grep -r "@svgr/core" package.json
# Check if it's installed in node_modules
find . -name "@svgr" -type d
Length of output: 62
| async loadInclude(path) { | ||
| // this.warn('loadInclude: ' + path); | ||
| path.endsWith('.hoo'); | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
修复 loadInclude 方法中的逻辑错误
loadInclude 方法中的 path.endsWith('.hoo') 表达式的结果未被使用，这会导致该方法总是返回 true。
建议修改为：
async loadInclude(path) {
// this.warn('loadInclude: ' + path);
- path.endsWith('.hoo');
- return true;
+ return path.endsWith('.hoo');
}📝 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 loadInclude(path) { | |
| // this.warn('loadInclude: ' + path); | |
| path.endsWith('.hoo'); | |
| return true; | |
| }, | |
| async loadInclude(path) { | |
| // this.warn('loadInclude: ' + path); | |
| return path.endsWith('.hoo'); | |
| }, |
| pub fn warn(&self, msg: String) { | ||
| println!("WARN: {}", msg) | ||
| } | ||
| #[napi] | ||
| pub fn error(&self, msg: String) { | ||
| println!("ERROR: {}", msg) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
改进错误和警告的处理机制
当前的 warn 和 error 方法实现过于简单，建议：
- 集成到项目的日志系统中
- 添加错误级别
- 支持结构化日志
#[napi]
impl PluginContext {
#[napi]
pub fn warn(&self, msg: String) {
- println!("WARN: {}", msg)
+ log::warn!("{}", msg)
}
#[napi]
pub fn error(&self, msg: String) {
- println!("ERROR: {}", msg)
+ log::error!("{}", msg)
}
}Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1741 +/- ##
=======================================
Coverage 54.82% 54.82%
=======================================
Files 180 180
Lines 18053 18053
=======================================
Hits 9897 9897
Misses 8156 8156 ☔ View full report in Codecov by Sentry. |
tasks
npx dumi buildin repo ant-design/ant-designref
#1737
why is load args undefined?
since the plugin item is patched twice. so it's only break when ssr is on, which means we will run mako twice.
how to reproduce origin problem?
how to fix this problem?
avoid patch twice in
packages/mako/src/index.ts.Summary by CodeRabbit
版本发布说明
新功能
PluginContext，增强插件开发能力emitFile、warn、errorunplugin生态系统插件文档更新
依赖更新
@iconify-json/mdi、unplugin-icons、unplugin-replace性能优化
本次更新主要聚焦于提升插件系统的灵活性和开发体验。