feat: native plugin init #1691
feat: native plugin init #1691
Conversation
|
Warning Rate limit exceeded@Jinbao1001 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 45 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. 📒 Files selected for processing (1)
Walkthrough本次更改主要涉及多个 Changes
Possibly related PRs
Suggested reviewers
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 (
|
WalkthroughThis pull request introduces support for Rust native plugins in the project. It includes changes to the configuration files to accommodate Rust plugins, updates to the plugin system to load and manage these plugins, and modifications to the build process to integrate Rust components. The changes also involve replacing existing memory allocators with Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| } | ||
|
|
||
| fn handler_create(input: TokenStream2) -> TokenStream2 { | ||
| let ast = parse2::<DeriveInput>(input).unwrap(); |
There was a problem hiding this comment.
The use of unwrap() can cause a panic if the input is not as expected. Consider handling the error more gracefully, perhaps by returning a Result or using expect() with a meaningful error message.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1691 +/- ##
==========================================
- Coverage 54.77% 54.58% -0.20%
==========================================
Files 181 180 -1
Lines 17981 17958 -23
==========================================
- Hits 9849 9802 -47
- Misses 8132 8156 +24 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (10)
crates/mako/src/ast.rs (1)
11-11: 建议为公开模块添加文档注释将
utils模块设置为公开访问符合插件系统的需求。不过，建议添加文档注释来说明：
- 该模块的用途
- 为什么需要公开访问
- 外部使用者应该注意的事项
建议添加如下文档注释：
+/// Utils module provides common utilities for AST manipulation. +/// This module is public to support native plugin integration. pub mod utils;packages/mako/src/rustPlugins/index.ts (3)
2-2: 函数参数类型可以更严格建议为插件选项定义更具体的接口类型，而不是使用
any。这样可以提供更好的类型安全性和代码提示。-export async function rustPluginResolver(plugins: Array<[string, any]>) { +interface PluginOptions { + // 根据实际需求定义选项接口 + [key: string]: unknown; +} +export async function rustPluginResolver(plugins: Array<[string, PluginOptions]>) {
6-10: 建议优化平台特定的导入逻辑当前的平台特定导入逻辑可以更简洁。可以使用统一的导入方式，减少代码重复。
- if (process.platform === 'win32') { - pluginPath = (await import(pathToFileURL(pluginPath).toString())).default; - } else { - pluginPath = await import(pluginPath).then((m) => m.default); - } + const importPath = process.platform === 'win32' + ? pathToFileURL(pluginPath).toString() + : pluginPath; + pluginPath = (await import(importPath)).default;
1-14: 建议添加 JSDoc 文档为了提高代码的可维护性和可读性，建议添加完整的 JSDoc 文档说明函数的用途、参数和返回值。
+/** + * 解析 Rust 原生插件列表 + * @param plugins - 插件配置数组，每个元素为 [插件名称, 插件选项] 的元组 + * @returns 解析后的插件路径和序列化选项数组 + * @throws {Error} 当插件解析或导入失败时抛出错误 + */ export async function rustPluginResolver(plugins: Array<[string, any]>) {Cargo.toml (1)
14-14: 建议固定依赖版本为了确保构建的一致性和可重现性，建议使用精确版本号。
-swc_malloc = "1.0.0" +swc_malloc = "=1.0.0"crates/mako/src/config/mako.config.default.json (1)
69-69: 配置更改看起来没有问题！在 experimental 部分添加
rustPlugins数组是合适的，这清楚地表明该功能仍处于实验阶段。默认值设置为空数组也是一个安全的选择。建议为这个新的配置项添加相关文档，说明其用途和使用方法。例如：
- 支持的插件格式
- 如何开发兼容的 Rust 插件
- 配置示例
packages/mako/src/binding.d.ts (1)
238-238: 建议完善类型定义和文档说明考虑以下几点改进建议：
any类型过于宽松，建议定义具体的插件配置接口以提供更好的类型安全性和开发体验- 缺少相关文档说明，建议添加 JSDoc 注释描述该特性的用途和配置方式
建议按照以下方式改进：
+ /** + * Rust 原生插件配置 + * @example + * rustPlugins: [ + * ['plugin-name', { config: 'value' }] + * ] + */ - rustPlugins?: Array<[string, any]>; + rustPlugins?: Array<[string, RustPluginConfig]>; + + /** Rust 插件配置接口 */ + interface RustPluginConfig { + // TODO: 根据实际需求定义具体配置项 + }packages/mako/binding.d.ts (1)
258-258: 建议：考虑为插件配置使用更具体的类型定义目前的类型定义
Array<[string, any]>中使用any可能过于宽松。建议：
- 为插件配置创建一个专门的接口，以提供更好的类型安全性和开发体验
- 考虑添加相关的类型文档，说明每个配置项的用途
示例改进：
interface RustPluginConfig { // 根据实际需求定义配置项 enabled?: boolean; options?: Record<string, unknown>; } // 然后更新类型定义 rustPlugins?: Array<[string, RustPluginConfig]>;crates/mako/src/config.rs (1)
235-255: 建议增强错误处理机制当前实现在处理实验性配置时可以更加健壮：
- 解析 JSON 时的错误处理可以更详细
- 建议添加对实验性配置内容的验证
建议按如下方式改进：
let result: Result<Value, serde_json::Error> = serde_json::from_str(default_config); -if let Ok(config) = result { +match result { + Ok(config) => { if let Some(experimental) = config.get("experimental") { + // 验证实验性配置的结构 + if !experimental.is_object() { + log::warn!("experimental 配置必须是一个对象"); + return c; + } overrides_json = Some( serde_json::to_string(&json!({ "experimental": experimental })) .unwrap(), ); } + } + Err(e) => { + log::warn!("解析默认配置失败: {}", e); + } };crates/mako_plugin_macro/src/lib.rs (1)
37-47: 改进参数名称匹配的精确度目前，代码判断参数名是否包含 "context"，这可能导致匹配到包含 "context" 的其他参数名，产生误判。建议将判断条件改为参数名等于 "context"，以提高匹配精确度，避免误判。
应用以下代码修改：
- i.ident.to_string().contains("context") + i.ident == "context"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
Cargo.toml(1 hunks)crates/binding/Cargo.toml(1 hunks)crates/binding/src/lib.rs(2 hunks)crates/mako/Cargo.toml(5 hunks)crates/mako/src/ast.rs(1 hunks)crates/mako/src/compiler.rs(4 hunks)crates/mako/src/config.rs(3 hunks)crates/mako/src/config/experimental.rs(2 hunks)crates/mako/src/config/mako.config.default.json(1 hunks)crates/mako/src/lib.rs(2 hunks)crates/mako/src/main.rs(1 hunks)crates/mako_plugin_macro/Cargo.toml(1 hunks)crates/mako_plugin_macro/src/lib.rs(1 hunks)packages/bundler-mako/index.js(1 hunks)packages/mako/binding.d.ts(1 hunks)packages/mako/src/binding.d.ts(1 hunks)packages/mako/src/index.ts(2 hunks)packages/mako/src/rustPlugins/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/mako_plugin_macro/Cargo.toml
🔇 Additional comments (24)
crates/mako/src/ast.rs (1)
11-11: 验证模块可见性的一致性
需要确认其他模块的可见性设置是否合理。目前大部分模块仍然是 pub(crate)，我们应该验证是否有其他模块也需要支持插件系统。
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information about the module visibility patterns in the codebase:
模块可见性设置合理，无需更改
当前的可见性设置是合理的:
utils模块设为pub是正确的，因为它包含了插件系统所需的公共工具- 其他大多数子模块保持
pub(crate)是合适的，因为它们是内部实现细节 - 整体架构遵循了清晰的可见性分层:
- 插件系统相关的模块(
plugin.rs、plugins/*)都是pub - 内部实现模块都是
pub(crate) - 工具类模块根据用途选择性地暴露
- 插件系统相关的模块(
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查 crates/mako 目录下所有的模块声明，以确保可见性设置的一致性
# 搜索所有的模块声明
rg "^pub(\(crate\))?\s+mod\s+" "crates/mako/src/"
# 搜索可能的插件相关代码，以确定哪些模块可能需要公开
rg -i "plugin|native" "crates/mako/src/"
Length of output: 33898
crates/binding/Cargo.toml (1)
18-18: 请验证 swc_malloc 在所有目标平台的兼容性和性能表现
从特定平台的内存分配器迁移到统一的 swc_malloc 是一个重要变更。建议：
- 在不同操作系统和架构上进行充分测试
- 评估对插件性能的潜在影响
- 确保内存分配策略的改变不会影响插件的稳定性
运行以下脚本检查项目中的目标平台支持：
✅ Verification successful
Let's analyze the workspace configuration and platform-specific code to better understand the memory allocator usage and platform support.
swc_malloc 的使用是安全且合适的
基于代码分析结果：
- swc_malloc 作为工作空间依赖被正确配置，版本为 1.0.0
- 项目中只有少量平台特定代码（仅针对 Windows 路径处理），说明内存分配器的改变不会影响跨平台兼容性
- swc_malloc 与 swc_core 一起被使用，这是标准的 SWC 工具链组合
- 内存分配器的变更仅限于必要的系统级组件
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查项目中定义的目标平台和 swc_malloc 的兼容性
# 搜索 build.rs 和 Cargo.toml 中的目标平台配置
rg -g '*.{rs,toml}' 'target_os|target_arch'
# 检查 swc_malloc 的版本和支持平台
cargo tree -i swc_malloc
Length of output: 274
Script:
#!/bin/bash
# Check workspace dependencies and swc_malloc usage
rg -g 'Cargo.toml' 'swc_malloc'
# Check for any platform-specific code or configurations
rg -g '*.rs' 'cfg\(target_' -A 2 -B 2
# Look for build configurations
fd 'build.rs' --exec cat {}
# Search for any platform-specific allocator code
rg -g '*.rs' 'allocator|malloc' -A 2 -B 2
Length of output: 12387
packages/mako/src/rustPlugins/index.ts (1)
11-11:
验证插件路径的有效性
在将插件路径添加到结果数组之前，建议验证 pluginPath 的有效性。
+ if (typeof pluginPath !== 'string') {
+ throw new Error(`插件 "${plugin}" 导出的默认值必须是字符串类型的路径`);
+ }
resolved.push([pluginPath, JSON.stringify(options)]);crates/mako/src/config/experimental.rs (1)
21-21: 建议为新增的配置字段添加文档
rust_plugins 字段作为实验性功能的一部分，需要清晰的文档说明其用途和使用方式。
建议添加字段文档：
+ /// 实验性的 Rust 原生插件配置列表
pub rust_plugins: Vec<RustPlugin>,另外，建议验证默认配置文件中是否已添加此字段：
crates/mako/src/lib.rs (2)
23-24: 验证依赖重导出的必要性
直接重导出 swc_core 和 swc_malloc 可能会:
- 暴露过多实现细节
- 增加 API 的不稳定性
- 使版本管理变得复杂
建议:
- 考虑封装必要的功能而不是直接重导出
- 添加文档说明重导出的原因和使用场景
13-13: 请确认模块可见性更改的必要性
将 module 模块改为公开可能会破坏封装性。建议:
- 确认是否所有内容都需要公开
- 考虑只公开必要的子模块或接口
- 添加文档注释说明公开的原因和用途
✅ Verification successful
将 module 模块改为公开是必要的
从代码分析来看，module 模块确实需要公开，因为:
- 它包含了许多核心类型如
ModuleId、Dependency、ModuleAst等，这些类型被整个项目广泛使用 - 这些类型在插件系统和编译流程中都是必需的
- 代码中有大量合法的跨模块引用，表明这是有意为之的架构设计
建议:
- 为 module 模块添加文档注释，说明其作为公共 API 的用途
- 仔细审查模块中的每个公开项，确保它们都是必要的
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 module 模块的使用情况
ast-grep --pattern 'use $$$module$$$'
Length of output: 47735
Script:
#!/bin/bash
# 检查 module 模块的导出和使用情况
fd -e rs -e toml . crates/mako/src/module/ -x cat {}
Length of output: 167
Script:
#!/bin/bash
# 检查 module 模块的定义和使用
rg "pub mod module" -A 5 crates/mako/src/lib.rs
rg "use (crate|super)::module" -t rust
Length of output: 6520
crates/mako/src/main.rs (2)
Line range hint 1-89: 建议完善插件系统架构设计
作为支持 Rust 原生插件的入口文件，建议考虑以下几点：
- 插件的生命周期管理
- 插件加载失败的错误处理机制
- 插件系统的初始化时机
需要我协助设计插件系统的架构文档或实现相关代码吗？
4-5: 验证内存分配器更改的影响
引入 swc_malloc 作为新的内存分配器替代了之前的平台特定配置。这个改动简化了内存管理策略，但需要确保在所有目标平台上都能正常工作。
✅ Verification successful
内存分配器配置变更已验证
引入 swc_malloc 作为外部依赖是合理的,因为:
- 项目同时依赖了
swc_malloc和tikv-jemallocator作为内存分配器 - 这些依赖在 Cargo.toml 中正确配置
- 在 main.rs 中通过
extern crate swc_malloc引入是标准做法
这个改动是对内存分配策略的优化,不会产生负面影响。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查项目中所有与内存分配相关的配置
rg -l "GlobalAlloc|global_allocator"
# 检查是否还有其他文件引用了旧的分配器
rg "mimalloc-rust|tikv_jemallocator"
Length of output: 254467
Script:
#!/bin/bash
# Check for global allocator configurations
rg -l "global_allocator|GlobalAlloc" --type rust
# Check for memory allocator dependencies in Cargo.toml files
rg -l "mimalloc-rust|tikv-jemallocator" --type toml
# Check for swc_malloc usage
rg -l "swc_malloc" --type rust
Length of output: 230
Script:
#!/bin/bash
# Check the content of relevant files to understand the memory allocator configuration
rg -A 5 "swc_malloc" crates/mako/src/main.rs
cat Cargo.toml | grep -A 5 "mimalloc-rust\|tikv-jemallocator\|swc_malloc"
Length of output: 387
crates/mako/Cargo.toml (4)
Line range hint 51-62: 验证 SWC 核心功能的兼容性
新增的 ecma_plugin_transform 和 swc_common 特性是支持原生插件所必需的，但需要确保版本兼容性。
建议：
- 确保这些新特性与现有的转换流程兼容
- 考虑添加相关的集成测试来验证插件系统的正确性
87-87: 检查插件系统相关依赖的完整性
新增的依赖（libloading、proc-macro2、quote、syn）构成了插件系统的基础架构。
建议：
libloading用于动态加载插件，建议添加错误处理机制- 宏相关的依赖（
proc-macro2、quote、syn）建议考虑是否需要在特性标志后面 - 考虑添加插件系统的文档说明
运行以下脚本检查依赖使用情况：
#!/bin/bash
# 检查新增依赖的使用情况
rg "libloading::" -t rust
rg "proc_macro2::" -t rustAlso applies to: 97-97, 100-100, 108-108
29-29: 检查 swc_malloc 的工作空间依赖配置
引入 swc_malloc 作为新的内存分配器是一个重要的改动。这个改动需要确保在整个工作空间中的一致性。
运行以下脚本来验证工作空间配置：
✅ Verification successful
工作空间中 swc_malloc 依赖配置正确
在工作空间中 swc_malloc 的配置完全一致：
- 根
Cargo.toml定义了版本 1.0.0 crates/mako和crates/binding都正确使用了 workspace 继承
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查工作空间中的 swc_malloc 配置
rg "swc_malloc" -g "Cargo.toml"
Length of output: 192
66-66: 确认 swc_emotion 版本固定的原因
将 swc_emotion 版本固定为 =0.72.22 可能会限制未来的更新。
运行以下脚本检查版本兼容性：
✅ Verification successful
版本固定是合理的做法
从代码库分析来看，这个版本固定是合理的：
- 项目中同时固定了其他重要依赖的版本，如
oxc_resolver = "=1.7.0"，并且在注释中说明了具体原因 swc_emotion作为 SWC 生态系统的一部分，与其他 SWC 相关依赖（如swc_core、swc_error_reporters、swc_node_comments）版本需要保持兼容- 这种精确的版本控制有助于确保构建的稳定性和可重现性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 swc_emotion 的版本历史和兼容性问题
gh api graphql -f query='
{
repository(owner: "swc-project", name: "swc") {
releases(first: 5, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
tagName
description
}
}
}
}
'
Length of output: 459
Script:
#!/bin/bash
# Check for any comments or documentation about version pinning
rg -g "*.{toml,md}" "swc_emotion.*0\.72\.22" -B 2 -A 2
# Check other dependencies versions in the same file
cat crates/mako/Cargo.toml
# Look for related dependencies
rg "swc.*=.*" -g "Cargo.toml"
Length of output: 4776
packages/mako/src/index.ts (2)
9-9: 导入语句正确添加！
新增的 rustPluginResolver 导入语句位置恰当，与现有代码风格保持一致。
47-51: 验证 Rust 插件与现有插件系统的集成
需要确认以下几点：
- Rust 插件是否需要在 watch 模式下进行特殊清理
- 是否需要在插件加载流程中添加特殊处理
运行以下脚本来验证插件系统的集成：
✅ Verification successful
Let me gather more information about the plugin system and watch mode handling.
Let me gather one final piece of information about the Rust plugin implementation.
Rust 插件系统集成验证结果
根据代码分析，Rust 插件系统已经具备完整的集成机制：
- 在 watch 模式下，插件系统通过
watch_changes钩子函数提供了标准的文件变更处理机制 - Rust 插件在编译器初始化时会被正确加载（见
crates/mako/src/compiler.rs） - 插件系统提供了统一的清理机制，不需要特殊处理
无需在插件加载流程中添加额外的特殊处理。现有的插件系统架构已经完全支持 Rust 插件的集成。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他文件引用了 rustPlugins
rg -l "rustPlugins"
# 检查插件系统相关的类型定义
ast-grep --pattern 'interface $_ {
$$$
plugins?: $_
$$$
}'
Length of output: 2223
Script:
#!/bin/bash
# 检查 watch 模式下的插件处理逻辑
rg -A 5 "watch.*mode"
# 检查插件系统的清理和卸载逻辑
rg -A 5 "(cleanup|unload|dispose).*plugin"
# 检查 Rust 插件的类型定义和接口
ast-grep --pattern 'type $_RustPlugin = {
$$$
}'
# 检查现有插件系统的加载逻辑
rg -A 10 "load.*plugin"
Length of output: 7509
Script:
#!/bin/bash
# 检查 Rust 插件的实现细节
rg -A 10 "rust_plugins" crates/
# 检查插件系统中的 watch 模式相关代码
rg -A 5 "fn watch" crates/
# 查看 Rust 插件的类型定义
ast-grep --pattern 'struct $_RustPlugin {
$$$
}'
Length of output: 4996
packages/mako/binding.d.ts (1)
258-258: 验证：确保实现与类型定义匹配
由于这是一个新的实验性功能，需要验证实现代码是否正确处理了这个类型定义。
✅ Verification successful
Let me check the implementation of rustPluginResolver to complete the verification.
类型定义与实现完全匹配，验证通过
rustPluginResolver 的实现完全符合类型定义：
- 接收的参数类型为
Array<[string, any]> - 返回的类型也是
Array<[string, any]> - 实现中正确处理了插件路径解析和选项序列化
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 rustPlugins 的实现是否符合类型定义
# 检查 rustPlugins 的使用情况
rg -A 5 "rustPlugins" --type ts
# 检查相关的插件加载逻辑
ast-grep --pattern 'function $_(params: BuildParams) {
$$$
}'
Length of output: 11722
Script:
#!/bin/bash
# Check rustPluginResolver implementation
rg -A 10 "rustPluginResolver" --type ts
# Check if there are any rust plugin related files
fd -e rs rust
Length of output: 2248
crates/binding/src/lib.rs (2)
158-158: 🛠️ Refactor suggestion
建议完善 Rust 插件的类型定义
rustPlugins 的类型定义过于宽松：
any类型不够严格，建议定义具体的插件配置接口- 缺少相关文档说明插件的使用方式
- 作为实验性功能，建议添加相关警告注释
建议修改为更严格的类型定义：
- rustPlugins?: Array<[string, any]>;
+ /**
+ * 实验性功能：Rust 原生插件支持
+ * @experimental 该功能仍在开发中，接口可能发生变化
+ */
+ rustPlugins?: Array<[string, RustPluginConfig]>;另外需要在相应位置添加 RustPluginConfig 接口定义。
#!/bin/bash
# 检查是否已实现插件加载逻辑
rg "rustPlugins?" --type rust3-4: 验证内存分配器的变更影响
引入 swc_malloc 作为新的内存分配器需要确保：
- 系统性能不会受到负面影响
- 与现有的内存管理机制兼容
- 在不同平台上都能正常工作
✅ Verification successful
内存分配器变更已正确集成
根据验证结果，swc_malloc 已在工作空间中正确配置：
- 在 workspace 依赖中定义了
swc_malloc = "1.0.0" - 多个关键 crate 都正确引入了
swc_malloc - 没有发现与旧的内存分配器（如 mimalloc）的冲突使用
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他文件仍在使用旧的分配器
rg "mimalloc|jemalloc" --type rust
Length of output: 195
Script:
#!/bin/bash
# 检查全局分配器的配置
rg "#\[global_allocator\]" --type rust -A 2
# 检查 swc_malloc 的使用情况
rg "use.*swc_malloc|extern.*swc_malloc" --type rust
# 检查 Cargo.toml 中的依赖
fd Cargo.toml --exec cat {}
Length of output: 6770
crates/mako/src/config.rs (2)
69-69: 导入变更正确！
引入 json 宏是合理的，它用于后续构建实验性配置的 JSON 对象。
276-284: 验证配置覆盖行为
需要确保配置覆盖的优先级符合预期，特别是在处理实验性的 Rust 原生插件配置时。
✅ Verification successful
配置覆盖行为已正确实现
从代码分析结果来看，配置源的加载顺序是正确的：
- 默认配置（DEFAULT_CONFIG）
- 配置文件（abs_config_file）
- 命令行配置（cli_config）
- 覆盖配置（overrides）
测试用例充分验证了不同配置源的优先级，包括：
- 默认配置测试
- 命令行参数覆盖测试
- 配置文件验证测试
实验性的 Rust 插件配置也能通过这个机制正确处理。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证配置文件中的实验性配置覆盖行为
# 搜索包含 experimental 配置的测试用例
rg -l "experimental.*rust" test/config/
# 检查配置解析相关的测试代码
ast-grep --pattern 'fn test_$_() {
$$$
Config::new($_)
$$$
}'
Length of output: 164
Script:
#!/bin/bash
# 搜索配置文件中的实验性配置相关代码
rg -l "experimental.*rust" crates/mako/src/
# 检查配置解析相关的测试代码
rg -l "test.*config" crates/mako/src/
# 查找配置相关的测试文件
fd "test.*config" crates/mako/src/
# 检查配置覆盖相关的测试代码
ast-grep --pattern 'fn test_$_() {
$$$
add_source($_)
$$$
}'
Length of output: 245
Script:
#!/bin/bash
# 检查 config.rs 中的测试用例
rg "fn test_.*config" -A 10 crates/mako/src/config.rs
# 检查 compiler.rs 中的实验性 Rust 配置使用
rg "experimental.*rust" -A 5 crates/mako/src/compiler.rs
# 检查配置覆盖的实现
rg "add_source" -A 5 crates/mako/src/config.rs
Length of output: 2950
crates/mako/src/compiler.rs (4)
9-9: 导入 libloading 库
添加 libloading::Library 的导入以支持动态加载 Rust 插件，这是必要的操作。
224-224: 将 config 参数设为可变
在 Compiler::new 方法中，将 config 参数设为可变（mut config: Config），以便在初始化编译器时修改配置。这是合理的，确保在方法内部可以适当调整配置。
244-253: 重复的安全性提醒：使用 unsafe 块加载插件
之前关于使用 unsafe 块可能导致未定义行为的评论仍然适用。请确保动态加载的 Rust 插件是可信的，并对 _plugin_create 函数的调用进行安全性验证，添加必要的检查或限制以降低潜在风险。
271-271: 确认插件的加载顺序
在第 271 行，将 external_plugins 扩展到 plugins 列表中。请确认外部插件的加载顺序符合预期，确保它们在内置插件之前正确加载，以避免功能冲突。
packages/bundler-mako/index.js (1)
665-665: 添加了 rustPlugins 配置项
在 experimental 配置中新增了 rustPlugins 属性，为支持 Rust 插件提供了扩展点。
| for (const [plugin, options] of plugins) { | ||
| let pluginPath = require.resolve(plugin); | ||
| if (process.platform === 'win32') { | ||
| pluginPath = (await import(pathToFileURL(pluginPath).toString())).default; | ||
| } else { | ||
| pluginPath = await import(pluginPath).then((m) => m.default); | ||
| } | ||
| resolved.push([pluginPath, JSON.stringify(options)]); |
There was a problem hiding this comment.
需要添加错误处理机制
当前实现缺少错误处理逻辑。插件解析和导入过程中可能会发生错误，建议添加 try-catch 块并提供有意义的错误信息。
for (const [plugin, options] of plugins) {
+ try {
let pluginPath = require.resolve(plugin);
if (process.platform === 'win32') {
pluginPath = (await import(pathToFileURL(pluginPath).toString())).default;
} else {
pluginPath = await import(pluginPath).then((m) => m.default);
}
resolved.push([pluginPath, JSON.stringify(options)]);
+ } catch (error) {
+ throw new Error(`插件 "${plugin}" 加载失败: ${error.message}`);
+ }
}📝 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.
| for (const [plugin, options] of plugins) { | |
| let pluginPath = require.resolve(plugin); | |
| if (process.platform === 'win32') { | |
| pluginPath = (await import(pathToFileURL(pluginPath).toString())).default; | |
| } else { | |
| pluginPath = await import(pluginPath).then((m) => m.default); | |
| } | |
| resolved.push([pluginPath, JSON.stringify(options)]); | |
| for (const [plugin, options] of plugins) { | |
| try { | |
| let pluginPath = require.resolve(plugin); | |
| if (process.platform === 'win32') { | |
| pluginPath = (await import(pathToFileURL(pluginPath).toString())).default; | |
| } else { | |
| pluginPath = await import(pluginPath).then((m) => m.default); | |
| } | |
| resolved.push([pluginPath, JSON.stringify(options)]); | |
| } catch (error) { | |
| throw new Error(`插件 "${plugin}" 加载失败: ${error.message}`); | |
| } | |
| } |
| #[derive(Deserialize, Serialize, Debug, Clone)] | ||
| pub struct RustPlugin { | ||
| pub path: String, | ||
| pub options: String, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议改进 RustPlugin 结构体的设计
当前实现存在以下需要改进的地方：
- 缺少结构体和字段的文档注释，建议添加以说明用途和预期值
options字段使用String类型可能过于简单，建议考虑使用更结构化的类型（如serde_json::Value）来支持更复杂的配置
建议按如下方式修改：
#[derive(Deserialize, Serialize, Debug, Clone)]
+/// 用于配置 Rust 原生插件的结构体
pub struct RustPlugin {
+ /// 插件的文件路径
pub path: String,
+ /// 插件的配置选项，支持 JSON 格式
- pub options: String,
+ pub options: serde_json::Value,
}📝 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.
| #[derive(Deserialize, Serialize, Debug, Clone)] | |
| pub struct RustPlugin { | |
| pub path: String, | |
| pub options: String, | |
| } | |
| #[derive(Deserialize, Serialize, Debug, Clone)] | |
| /// 用于配置 Rust 原生插件的结构体 | |
| pub struct RustPlugin { | |
| /// 插件的文件路径 | |
| pub path: String, | |
| /// 插件的配置选项，支持 JSON 格式 | |
| pub options: serde_json::Value, | |
| } |
| const rustPlugins = params.config.experimental?.rustPlugins; | ||
| if (rustPlugins) { | ||
| params.config.experimental!.rustPlugins = | ||
| await rustPluginResolver(rustPlugins); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增强错误处理和类型安全性
当前实现有以下几点需要注意：
- 缺少对
rustPluginResolver执行失败的错误处理 - 直接修改
params.config.experimental可能导致类型不安全 - 建议添加日志记录以便于调试
建议按照以下方式重构：
const rustPlugins = params.config.experimental?.rustPlugins;
if (rustPlugins) {
+ try {
params.config.experimental!.rustPlugins =
await rustPluginResolver(rustPlugins);
+ console.log('Rust plugins successfully resolved');
+ } catch (error) {
+ console.error('Failed to resolve Rust plugins:', error);
+ throw new Error(`Rust plugin resolution failed: ${error.message}`);
+ }
}📝 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 rustPlugins = params.config.experimental?.rustPlugins; | |
| if (rustPlugins) { | |
| params.config.experimental!.rustPlugins = | |
| await rustPluginResolver(rustPlugins); | |
| } | |
| const rustPlugins = params.config.experimental?.rustPlugins; | |
| if (rustPlugins) { | |
| try { | |
| params.config.experimental!.rustPlugins = | |
| await rustPluginResolver(rustPlugins); | |
| console.log('Rust plugins successfully resolved'); | |
| } catch (error) { | |
| console.error('Failed to resolve Rust plugins:', error); | |
| throw new Error(`Rust plugin resolution failed: ${error.message}`); | |
| } | |
| } |
crates/mako_plugin_macro/src/lib.rs
Outdated
| #[no_mangle] | ||
| pub fn _plugin_create(option: std::string::String) -> std::sync::Arc<dyn Plugin> { | ||
| std::sync::Arc::new(#struct_name::new(option)) | ||
| } |
There was a problem hiding this comment.
缺少对 Plugin trait 的引用
代码中使用了 dyn Plugin，但当前作用域中未引入 Plugin trait，可能会导致编译错误。请考虑在文件顶部添加对 Plugin trait` 的引用。
应用以下代码修改：
+ use mako::plugin::Plugin;Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
crates/mako/src/config.rs (1)
246-256: 建议优化错误处理和代码结构当前代码的嵌套结构较深，建议重构以提高可读性和维护性。
建议按照以下方式重构：
- let result: Result<Value, serde_json::Error> = serde_json::from_str(default_config); - if let Ok(config) = result { - if let Some(experimental) = config.get("experimental") { - overrides_json = Some( - serde_json::to_string(&json!({ - "experimental": experimental - })) - .unwrap(), - ); - } - }; + overrides_json = serde_json::from_str::<Value>(default_config) + .ok() + .and_then(|config| config.get("experimental")) + .map(|experimental| { + serde_json::to_string(&json!({ + "experimental": experimental + })) + .expect("Failed to serialize experimental config") + });crates/binding/src/lib.rs (1)
161-161: 建议改进类型定义以增强类型安全性当前
rustPlugins的类型定义使用了any，这可能导致运行时类型错误。建议定义一个更具体的接口来描述插件配置。建议修改为:
- rustPlugins?: Array<[string, any]>; + rustPlugins?: Array<[string, { + path: string; + options: Record<string, unknown>; + }]>;crates/mako/src/compiler.rs (2)
9-9: 确保 libloading 版本的安全性建议在
Cargo.toml中明确指定libloading的版本，以避免潜在的安全问题。
271-271: 建议调整插件加载顺序的处理方式当前实现中，外部插件被添加到内置插件之前，这可能导致某些依赖内置插件功能的外部插件无法正常工作。建议：
- 允许插件定义依赖关系
- 根据依赖关系对插件进行排序
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
crates/binding/src/lib.rs(2 hunks)crates/mako/src/compiler.rs(3 hunks)crates/mako/src/config.rs(3 hunks)crates/mako/src/config/experimental.rs(2 hunks)crates/mako/src/config/mako.config.default.json(1 hunks)crates/mako_plugin_macro/src/lib.rs(1 hunks)packages/mako/binding.d.ts(1 hunks)packages/mako/src/binding.d.ts(1 hunks)packages/mako/src/rustPlugins/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/mako/src/config/mako.config.default.json
- packages/mako/src/rustPlugins/index.ts
- packages/mako/binding.d.ts
- packages/mako/src/binding.d.ts
- crates/mako/src/config/experimental.rs
- crates/mako_plugin_macro/src/lib.rs
🔇 Additional comments (5)
crates/mako/src/config.rs (4)
69-69: 导入更改看起来没有问题！
引入 serde_json::json 宏是合理的，它为后续处理实验性配置提供了必要支持。
236-236: 变量声明合理！
overrides_json 的类型选择和命名都很恰当，使用 Option<String> 可以很好地处理可能存在的覆盖配置。
277-285: 配置覆盖逻辑实现合理！
使用 builder 模式来添加覆盖配置的实现清晰且符合最佳实践。
Line range hint 236-285: 请验证实验性功能的文档和兼容性
新增的实验性配置处理机制可能会影响现有的配置行为。
运行以下脚本来验证文档和测试覆盖：
✅ Verification successful
实验性功能配置已得到完整文档和测试支持
通过分析发现：
- 实验性功能已在中英文文档中详细说明（docs/config.zh-CN.md 和 docs/config.md）
- 所有实验性功能都有明确的类型定义和默认值配置
- 代码中有相应的测试用例和配置示例
- 实验性功能的变更都有完整的 CHANGELOG 记录
实验性功能包括：
- detectLoop：循环依赖检测
- requireContext：require.context 支持
- webpackSyntaxValidate：webpack 语法验证
- magicComment：魔法注释支持
- 其他内部实验性功能
配置处理机制安全可靠，不会影响现有功能的稳定性。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查实验性功能的文档
echo "检查实验性功能文档..."
fd -t f "README" -X rg -l "experimental"
# 检查相关测试用例
echo "检查测试覆盖..."
rg -l "test.*experimental" "crates/mako/src"
# 检查配置模式的使用示例
echo "检查配置示例..."
fd -t f "example" -X rg -l "experimental"
Length of output: 450
Script:
#!/bin/bash
# 搜索所有包含 experimental 的配置相关文件
echo "搜索配置文件中的实验性功能..."
rg -l "experimental" "crates/mako/src"
# 检查配置结构定义
echo "检查配置结构..."
ast-grep --pattern 'struct $_ {
$$$
experimental: $_,
$$$
}'
# 搜索实验性功能的具体使用
echo "搜索实验性功能的使用..."
rg "experimental" -A 3 -B 3
# 检查配置相关的测试
echo "检查配置测试..."
fd -t f "test" -X rg -l "config.*experimental|experimental.*config"
Length of output: 26059
crates/mako/src/compiler.rs (1)
244-253: 关于不安全代码块的使用
使用 unsafe 块可能导致未定义行为。确保加载的 Rust 插件是可信的，并且 _plugin_create 函数在给定选项的情况下是安全的。建议添加检查或约束以降低潜在风险。
crates/mako/src/compiler.rs
Outdated
| unsafe { | ||
| for rust_plugin in config.experimental.rust_plugins.clone() { | ||
| let lib = Arc::new(Library::new(rust_plugin.path)?); | ||
| let plugin_create_fn: libloading::Symbol< | ||
| unsafe extern "C" fn(option: String) -> Arc<dyn Plugin>, | ||
| > = lib.get(b"_plugin_create").unwrap(); | ||
| let plugin = plugin_create_fn(rust_plugin.options); | ||
| external_plugins.push(plugin); | ||
| } | ||
| } |
There was a problem hiding this comment.
需要加强动态库加载的安全性和错误处理
当前实现存在以下问题：
- 缺少对加载库的有效性验证
- 错误处理不够完善
- 未对插件接口进行版本检查
建议重构为：
+ fn load_rust_plugin(plugin_path: String, options: String) -> Result<Arc<dyn Plugin>> {
+ // 验证插件文件是否存在且具有正确的格式
+ if !std::path::Path::new(&plugin_path).exists() {
+ return Err(anyhow!("Plugin file not found: {}", plugin_path));
+ }
+
+ unsafe {
+ let lib = Library::new(&plugin_path)
+ .map_err(|e| anyhow!("Failed to load plugin {}: {}", plugin_path, e))?;
+
+ // 验证插件版本兼容性
+ let version_fn: libloading::Symbol<unsafe extern "C" fn() -> u32> =
+ lib.get(b"_plugin_version")
+ .map_err(|_| anyhow!("Plugin {} is missing version information", plugin_path))?;
+
+ let version = version_fn();
+ if version != PLUGIN_API_VERSION {
+ return Err(anyhow!("Plugin {} version mismatch", plugin_path));
+ }
+
+ // 加载插件创建函数
+ let create_fn: libloading::Symbol<unsafe extern "C" fn(String) -> Arc<dyn Plugin>> =
+ lib.get(b"_plugin_create")
+ .map_err(|_| anyhow!("Plugin {} is invalid", plugin_path))?;
+
+ Ok(create_fn(options))
+ }
+ }
let mut external_plugins: Vec<Arc<dyn Plugin>> = vec![];
for rust_plugin in config.experimental.rust_plugins.clone() {
- unsafe {
- let lib = Arc::new(Library::new(rust_plugin.path)?);
- let plugin_create_fn: libloading::Symbol<
- unsafe extern "C" fn(option: String) -> Arc<dyn Plugin>,
- > = lib.get(b"_plugin_create").unwrap();
- let plugin = plugin_create_fn(rust_plugin.options);
- external_plugins.push(plugin);
- }
+ match load_rust_plugin(rust_plugin.path, rust_plugin.options) {
+ Ok(plugin) => external_plugins.push(plugin),
+ Err(e) => {
+ eprintln!("Warning: Failed to load plugin: {}", e);
+ continue;
+ }
+ }
}📝 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.
| unsafe { | |
| for rust_plugin in config.experimental.rust_plugins.clone() { | |
| let lib = Arc::new(Library::new(rust_plugin.path)?); | |
| let plugin_create_fn: libloading::Symbol< | |
| unsafe extern "C" fn(option: String) -> Arc<dyn Plugin>, | |
| > = lib.get(b"_plugin_create").unwrap(); | |
| let plugin = plugin_create_fn(rust_plugin.options); | |
| external_plugins.push(plugin); | |
| } | |
| } | |
| fn load_rust_plugin(plugin_path: String, options: String) -> Result<Arc<dyn Plugin>> { | |
| // 验证插件文件是否存在且具有正确的格式 | |
| if !std::path::Path::new(&plugin_path).exists() { | |
| return Err(anyhow!("Plugin file not found: {}", plugin_path)); | |
| } | |
| unsafe { | |
| let lib = Library::new(&plugin_path) | |
| .map_err(|e| anyhow!("Failed to load plugin {}: {}", plugin_path, e))?; | |
| // 验证插件版本兼容性 | |
| let version_fn: libloading::Symbol<unsafe extern "C" fn() -> u32> = | |
| lib.get(b"_plugin_version") | |
| .map_err(|_| anyhow!("Plugin {} is missing version information", plugin_path))?; | |
| let version = version_fn(); | |
| if version != PLUGIN_API_VERSION { | |
| return Err(anyhow!("Plugin {} version mismatch", plugin_path)); | |
| } | |
| // 加载插件创建函数 | |
| let create_fn: libloading::Symbol<unsafe extern "C" fn(String) -> Arc<dyn Plugin>> = | |
| lib.get(b"_plugin_create") | |
| .map_err(|_| anyhow!("Plugin {} is invalid", plugin_path))?; | |
| Ok(create_fn(options)) | |
| } | |
| } | |
| let mut external_plugins: Vec<Arc<dyn Plugin>> = vec![]; | |
| for rust_plugin in config.experimental.rust_plugins.clone() { | |
| match load_rust_plugin(rust_plugin.path, rust_plugin.options) { | |
| Ok(plugin) => external_plugins.push(plugin), | |
| Err(e) => { | |
| eprintln!("Warning: Failed to load plugin: {}", e); | |
| continue; | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/mako_plugin_macro/src/lib.rs (1)
13-23: 🛠️ Refactor suggestion改进错误处理并添加函数文档
- 使用
unwrap()处理错误可能导致编译器 panic，这是一个重复出现的问题。- 生成的
_plugin_create函数缺少文档注释。建议应用以下修改：
fn handler_create(input: TokenStream2) -> TokenStream2 { - let ast = parse2::<DeriveInput>(input).unwrap(); + let ast = parse2::<DeriveInput>(input) + .expect("解析派生宏输入失败，请确保语法正确"); let struct_name = &ast.ident; let ts = quote! { + /// 创建插件实例的工厂函数 + /// + /// # 参数 + /// * `option` - 插件的配置选项 + /// + /// # 返回值 + /// 返回包装在 Arc 中的插件实例 #[no_mangle] pub fn _plugin_create(option: serde_json::Value) -> std::sync::Arc<dyn Plugin> { std::sync::Arc::new(#struct_name::new(option)) } }; ts }
🧹 Nitpick comments (3)
crates/mako/src/compiler.rs (1)
272-272: 建议添加插件顺序的说明文档当前实现中外部插件在内置插件之前添加，这可能会影响插件的处理顺序。建议：
- 添加注释说明插件加载顺序的要求
- 考虑提供配置选项允许用户控制插件加载顺序
crates/mako_plugin_macro/Cargo.toml (1)
7-11: 建议优化依赖版本约束当前的依赖配置基本合理，但建议：
- 为了更好的安全性和可预测性，建议使用更精确的版本约束
- 对于proc-macro2和quote，建议使用类似
^1.0.0的形式[dependencies] -proc-macro2 = "1" -quote = "1" +proc-macro2 = "^1.0.0" +quote = "^1.0.0" serde_json = { workspace = true } syn = { version = "2", features = ["full"] }crates/mako_plugin_macro/src/lib.rs (1)
1-7: 建议添加不稳定特性的文档说明代码使用了不稳定的
box_patterns特性。建议添加注释说明使用该特性的原因，以及可能的稳定化时间线。这将有助于后续维护和升级。+// 使用不稳定特性 box_patterns 用于在 handle_local 中进行 box 模式匹配 +// 跟踪问题: https://github.com/rust-lang/rust/issues/29641 #![feature(box_patterns)]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/mako/src/ast.rs(1 hunks)crates/mako/src/compiler.rs(3 hunks)crates/mako/src/config.rs(4 hunks)crates/mako/src/config/experimental.rs(2 hunks)crates/mako_plugin_macro/Cargo.toml(1 hunks)crates/mako_plugin_macro/src/lib.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/mako/src/ast.rs
- crates/mako/src/config/experimental.rs
🔇 Additional comments (5)
crates/mako/src/compiler.rs (1)
245-254:
需要加强动态库加载的安全性和错误处理
当前实现存在以下安全隐患：
- 使用了不安全的代码块但缺少必要的安全检查
- 直接使用unwrap()处理错误，可能导致程序崩溃
- 缺少插件版本兼容性验证
- 没有对插件接口进行验证
建议重构为：
- unsafe {
- for rust_plugin in config.experimental.rust_plugins.clone() {
- let lib = Arc::new(Library::new(rust_plugin.path)?);
- let plugin_create_fn: libloading::Symbol<
- unsafe extern "C" fn(option: Value) -> Arc<dyn Plugin>,
- > = lib.get(b"_plugin_create").unwrap();
- let plugin = plugin_create_fn(rust_plugin.options);
- external_plugins.push(plugin);
- }
- }
+ for rust_plugin in config.experimental.rust_plugins.clone() {
+ match unsafe { Self::load_plugin(&rust_plugin) } {
+ Ok(plugin) => external_plugins.push(plugin),
+ Err(e) => {
+ eprintln!("Failed to load plugin {}: {}", rust_plugin.path, e);
+ continue;
+ }
+ }
+ }
// 新增辅助函数
+ unsafe fn load_plugin(rust_plugin: &RustPlugin) -> Result<Arc<dyn Plugin>> {
+ // 验证插件文件
+ if !std::path::Path::new(&rust_plugin.path).exists() {
+ return Err(anyhow!("Plugin file not found: {}", rust_plugin.path));
+ }
+
+ let lib = Library::new(&rust_plugin.path)
+ .map_err(|e| anyhow!("Failed to load plugin: {}", e))?;
+
+ // 验证插件版本
+ let version_fn: Symbol<unsafe extern "C" fn() -> u32> =
+ lib.get(b"_plugin_version")
+ .map_err(|_| anyhow!("Invalid plugin: missing version"))?;
+
+ let version = version_fn();
+ if version != PLUGIN_API_VERSION {
+ return Err(anyhow!("Plugin version mismatch"));
+ }
+
+ // 加载创建函数
+ let create_fn: Symbol<unsafe extern "C" fn(Value) -> Arc<dyn Plugin>> =
+ lib.get(b"_plugin_create")
+ .map_err(|_| anyhow!("Invalid plugin: missing create function"))?;
+
+ Ok(create_fn(rust_plugin.options))
+ }Likely invalid or redundant comment.
crates/mako_plugin_macro/Cargo.toml (3)
1-6: 包元数据配置正确！
包名称、版本号、Rust版本和许可证的配置都符合Rust生态系统的最佳实践。
13-14: 过程宏配置正确！
proc-macro = true的配置正确指定了这是一个过程宏crate。
1-14: 验证过程宏的集成使用情况
作为新增的原生插件系统的一部分，我们需要验证此过程宏在代码库中的使用情况。
crates/mako_plugin_macro/src/lib.rs (1)
52-54: 验证全局状态操作的线程安全性
使用 GLOBALS.set 修改全局状态可能会在并发场景下产生问题。需要确保：
- 全局状态的修改是线程安全的
- 不会出现死锁或竞态条件
- 资源在操作完成后被正确释放
✅ Verification successful
Let me analyze the search results to verify the thread safety concerns of GLOBALS.set.
让我们再搜索一下 swc_core 中 GLOBALS 的具体实现：
基于已有的搜索结果，我可以生成最终的回应了。
GLOBALS.set 的使用是线程安全的
GLOBALS.set 是从 swc_core 继承的线程局部存储(thread-local storage)机制，具有以下特点：
- 它使用了 Rust 的线程局部存储，每个线程都有自己独立的存储空间
- 所有调用都遵循相同的模式：传入一个闭包并在作用域结束时自动清理
- 代码库中大量的测试用例都在并发环境下使用这个机制
因此，原始的担忧是不必要的，这个实现是线程安全的。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索 GLOBALS 的使用情况和相关的同步机制
rg -A 5 "GLOBALS" --type rust
# 检查是否有相关的测试用例
fd -e rs test_ --exec grep -l "GLOBALS"
Length of output: 46140
Script:
# Search for GLOBALS type definition and implementation
ast-grep --pattern 'struct GLOBALS'
# Search for any synchronization primitives used with GLOBALS
rg -B 2 -A 5 'impl.*GLOBALS'
# Look for any documentation about thread safety
rg -l '///' | xargs rg -A 5 'GLOBALS'
Length of output: 1642
Script:
# Search for GLOBALS in swc_core dependency documentation
rg -l "swc_core.*GLOBALS" | xargs rg -B 3 -A 3 "GLOBALS"
# Look for any thread-local or synchronization related types used with GLOBALS
ast-grep --pattern 'use $$$thread$$$;'
# Search for any panic or safety related comments near GLOBALS usage
rg -B 3 -A 3 "// .*safety|// .*thread|// .*panic" --type rust | grep -C 5 "GLOBALS"
Length of output: 52780
Script:
# Search for GLOBALS in swc_core crate documentation
rg -A 5 "pub.*GLOBALS" --glob "Cargo.toml" --glob "*.rs"
# Look for thread-local storage usage
ast-grep --pattern 'thread_local!'
# Search for any synchronization mechanisms around GLOBALS
rg -B 3 -A 3 "set.*\&.*globals" --type rust
Length of output: 31717
Script:
# 查找 swc_core 依赖中的 GLOBALS 定义
rg -p "GLOBALS.*=.*thread_local" --type rust
# 查找 GLOBALS 相关的错误处理
rg "GLOBALS.*panic|GLOBALS.*error" --type rust
# 查找 GLOBALS 的类型定义
ast-grep --pattern 'type $_ = GLOBALS'
Length of output: 229
| let result: Result<Value, serde_json::Error> = serde_json::from_str(default_config); | ||
| if let Ok(config) = result { | ||
| if let Some(rust_plugins) = config | ||
| .get("experimental") | ||
| .and_then(|experimental| experimental.get("rustPlugins")) | ||
| { | ||
| overrides_json = Some(json!({ "rust_plugins": rust_plugins })); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
需要改进配置解析的健壮性
当前实现存在以下问题：
- 使用unwrap()直接访问JSON数据，缺少错误处理
- 缺少对rust_plugins配置结构的验证
- 缺少实验性功能的文档说明
建议修改为：
- if let Ok(config) = result {
- if let Some(rust_plugins) = config
- .get("experimental")
- .and_then(|experimental| experimental.get("rustPlugins"))
- {
- overrides_json = Some(json!({ "rust_plugins": rust_plugins }));
- }
- };
+ if let Ok(config) = result {
+ if let Some(experimental) = config.get("experimental") {
+ if let Some(rust_plugins) = experimental.get("rustPlugins") {
+ // 验证rust_plugins的结构
+ if !rust_plugins.is_array() {
+ return Err(anyhow!("rustPlugins must be an array"));
+ }
+
+ // 验证每个插件的配置
+ for plugin in rust_plugins.as_array().unwrap() {
+ if !plugin.is_object()
+ || plugin.get("path").is_none()
+ || !plugin.get("path").unwrap().is_string() {
+ return Err(anyhow!("Invalid plugin configuration"));
+ }
+ }
+
+ overrides_json = Some(json!({ "rust_plugins": rust_plugins }));
+ }
+ }
+ };另外建议添加以下文档：
/// 实验性功能：Rust原生插件支持
///
/// # 配置示例
/// ```json
/// {
/// "experimental": {
/// "rustPlugins": [
/// {
/// "path": "./plugins/my-plugin.so",
/// "options": { ... }
/// }
/// ]
/// }
/// }
/// ```
Also applies to: 283-287
| fn handle_local(_attr: TokenStream2, item: TokenStream2) -> TokenStream2 { | ||
| let ast = parse2::<ItemImpl>(item).unwrap(); | ||
| let items = &ast.items; | ||
| let mut expand_items: Vec<ImplItem> = vec![]; | ||
|
|
||
| items.iter().for_each(|impl_item| { | ||
| if let ImplItem::Fn(f) = impl_item { | ||
| let body = &f.block; | ||
|
|
||
| let has_context = f.sig.inputs.iter().any(|f| { | ||
| if let FnArg::Typed(PatType { | ||
| pat: box Pat::Ident(i), | ||
| .. | ||
| }) = f | ||
| { | ||
| i.ident.to_string().contains("context") | ||
| } else { | ||
| false | ||
| } | ||
| }); | ||
| let block = if has_context { | ||
| parse_quote!({ | ||
| mako::swc_core::common::GLOBALS.set(&context.meta.script.globals, || | ||
| #body | ||
| ) | ||
| }) | ||
| } else { | ||
| parse_quote!( | ||
| #body | ||
| ) | ||
| }; | ||
|
|
||
| expand_items.push(ImplItem::Fn(ImplItemFn { block, ..f.clone() })); | ||
| } | ||
| }); | ||
| let a = ItemImpl { | ||
| items: expand_items, | ||
| ..ast | ||
| }; | ||
| a.into_token_stream() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
优化代码结构和错误处理
当前实现存在以下问题：
- 错误处理使用了
unwrap() - 上下文检测逻辑较为复杂
- 缺少函数级别的文档说明
建议进行如下改进：
+/// 处理本地上下文属性宏
+///
+/// 检测函数参数中是否包含 context，并在必要时包装函数体
fn handle_local(_attr: TokenStream2, item: TokenStream2) -> TokenStream2 {
- let ast = parse2::<ItemImpl>(item).unwrap();
+ let ast = parse2::<ItemImpl>(item)
+ .expect("解析 impl 块失败，请确保语法正确");
let items = &ast.items;
let mut expand_items: Vec<ImplItem> = vec![];
+ // 提取上下文检测逻辑到独立函数
+ fn has_context_param(arg: &FnArg) -> bool {
+ matches!(arg,
+ FnArg::Typed(PatType {
+ pat: box Pat::Ident(i),
+ ..
+ }) if i.ident.to_string().contains("context")
+ )
+ }
items.iter().for_each(|impl_item| {
if let ImplItem::Fn(f) = impl_item {
let body = &f.block;
- let has_context = f.sig.inputs.iter().any(|f| {
- if let FnArg::Typed(PatType {
- pat: box Pat::Ident(i),
- ..
- }) = f
- {
- i.ident.to_string().contains("context")
- } else {
- false
- }
- });
+ let has_context = f.sig.inputs.iter().any(has_context_param);📝 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.
| fn handle_local(_attr: TokenStream2, item: TokenStream2) -> TokenStream2 { | |
| let ast = parse2::<ItemImpl>(item).unwrap(); | |
| let items = &ast.items; | |
| let mut expand_items: Vec<ImplItem> = vec![]; | |
| items.iter().for_each(|impl_item| { | |
| if let ImplItem::Fn(f) = impl_item { | |
| let body = &f.block; | |
| let has_context = f.sig.inputs.iter().any(|f| { | |
| if let FnArg::Typed(PatType { | |
| pat: box Pat::Ident(i), | |
| .. | |
| }) = f | |
| { | |
| i.ident.to_string().contains("context") | |
| } else { | |
| false | |
| } | |
| }); | |
| let block = if has_context { | |
| parse_quote!({ | |
| mako::swc_core::common::GLOBALS.set(&context.meta.script.globals, || | |
| #body | |
| ) | |
| }) | |
| } else { | |
| parse_quote!( | |
| #body | |
| ) | |
| }; | |
| expand_items.push(ImplItem::Fn(ImplItemFn { block, ..f.clone() })); | |
| } | |
| }); | |
| let a = ItemImpl { | |
| items: expand_items, | |
| ..ast | |
| }; | |
| a.into_token_stream() | |
| } | |
| /// 处理本地上下文属性宏 | |
| /// | |
| /// 检测函数参数中是否包含 context，并在必要时包装函数体 | |
| fn handle_local(_attr: TokenStream2, item: TokenStream2) -> TokenStream2 { | |
| let ast = parse2::<ItemImpl>(item) | |
| .expect("解析 impl 块失败，请确保语法正确"); | |
| let items = &ast.items; | |
| let mut expand_items: Vec<ImplItem> = vec![]; | |
| // 提取上下文检测逻辑到独立函数 | |
| fn has_context_param(arg: &FnArg) -> bool { | |
| matches!(arg, | |
| FnArg::Typed(PatType { | |
| pat: box Pat::Ident(i), | |
| .. | |
| }) if i.ident.to_string().contains("context") | |
| ) | |
| } | |
| items.iter().for_each(|impl_item| { | |
| if let ImplItem::Fn(f) = impl_item { | |
| let body = &f.block; | |
| let has_context = f.sig.inputs.iter().any(has_context_param); | |
| let block = if has_context { | |
| parse_quote!({ | |
| mako::swc_core::common::GLOBALS.set(&context.meta.script.globals, || | |
| #body | |
| ) | |
| }) | |
| } else { | |
| parse_quote!( | |
| #body | |
| ) | |
| }; | |
| expand_items.push(ImplItem::Fn(ImplItemFn { block, ..f.clone() })); | |
| } | |
| }); | |
| let a = ItemImpl { | |
| items: expand_items, | |
| ..ast | |
| }; | |
| a.into_token_stream() | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
crates/binding/src/lib.rs(2 hunks)crates/mako/src/compiler.rs(3 hunks)crates/mako/src/config.rs(4 hunks)crates/mako/src/config/mako.config.default.json(1 hunks)packages/mako/binding.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/mako/binding.d.ts
- crates/mako/src/config/mako.config.default.json
- crates/mako/src/compiler.rs
🔇 Additional comments (2)
crates/mako/src/config.rs (2)
250-258: 需要改进配置解析的健壮性
当前实现存在与之前相同的问题，包括使用unwrap()可能导致panic等。
284-291:
建议增强错误处理机制
当前实现存在以下问题：
- 使用
unwrap()直接访问 JSON 数据，可能导致运行时崩溃 - 缺少对插件配置的结构验证
建议修改为：
- if let Some(overrides) = overrides_json {
- let rust_plugins: Vec<RustPlugin> =
- serde_json::from_value(overrides.get("rust_plugins").unwrap().clone())?;
- config.experimental.rust_plugins = rust_plugins;
- }
+ if let Some(overrides) = overrides_json {
+ if let Some(plugins_value) = overrides.get("rust_plugins") {
+ match serde_json::from_value::<Vec<RustPlugin>>(plugins_value.clone()) {
+ Ok(rust_plugins) => {
+ config.experimental.rust_plugins = rust_plugins;
+ }
+ Err(e) => {
+ return Err(anyhow!("Invalid rust_plugins configuration: {}", e));
+ }
+ }
+ }
+ }Likely invalid or redundant comment.
| @@ -168,6 +158,7 @@ pub struct BuildParams { | |||
| }; | |||
| experimental?: { | |||
| webpackSyntaxValidate?: string[]; | |||
| rustPlugins?: Array<[string, any]>; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议加强类型定义和文档说明
当前的 rustPlugins 类型定义过于宽松：
- 使用
any类型不利于类型安全 - 缺少对插件配置结构的文档说明
建议修改为：
- rustPlugins?: Array<[string, any]>;
+ rustPlugins?: Array<{
+ // 插件的路径
+ path: string;
+ // 插件的配置选项
+ options?: {
+ [key: string]: string | number | boolean;
+ };
+ }>;同时建议添加相关文档说明插件的配置格式和用法。
📝 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.
| rustPlugins?: Array<[string, any]>; | |
| rustPlugins?: Array<{ | |
| // 插件的路径 | |
| path: string; | |
| // 插件的配置选项 | |
| options?: { | |
| [key: string]: string | number | boolean; | |
| }; | |
| }>; |
support rust native plugin
Summary by CodeRabbit
新功能
rustPlugins配置选项，允许用户在构建过程中集成 Rust 插件。rustPluginResolver，用于解析和加载 Rust 插件。RustPlugin结构体，扩展了实验性配置选项。文档
mako.config.default.json和 TypeScript 类型定义，以支持新的 Rust 插件配置。样式