-
Notifications
You must be signed in to change notification settings - Fork 73
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
[WIP] feat: native plugin init #1691
base: master
Are you sure you want to change the base?
Conversation
Walkthrough本次更改主要涉及多个 Changes
Possibly related PRs
Suggested reviewers
Warning Rate limit exceeded@Jinbao1001 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 39 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. 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
|
@@ -238,6 +239,19 @@ impl Compiler { | |||
if let Some(extra_plugins) = extra_plugins { | |||
plugins.extend(extra_plugins); | |||
} | |||
|
|||
let mut external_plugins: Vec<Arc<dyn Plugin>> = vec![]; | |||
unsafe { |
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.
Using unsafe
blocks can lead to undefined behavior if not handled correctly. Ensure that the Rust plugins being loaded are trusted and that the _plugin_create
function is safe to call with the provided options. Consider adding checks or constraints to mitigate potential risks.
} | ||
|
||
fn handler_create(input: TokenStream2) -> TokenStream2 { | ||
let ast = parse2::<DeriveInput>(input).unwrap(); |
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.
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 55.27% 55.24% -0.04%
==========================================
Files 175 175
Lines 17700 17730 +30
==========================================
+ Hits 9784 9795 +11
- Misses 7916 7935 +19 ☔ 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: 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.lock
is 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 rust
Also 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 rust
3-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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要添加错误处理机制
当前实现缺少错误处理逻辑。插件解析和导入过程中可能会发生错误,建议添加 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}`); | |
} | |
} |
swc_malloc = "1.0.0" | ||
tikv-jemallocator = { version = "=0.5.4", features = ["disable_initial_exec_tls"] } |
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.
💡 Codebase verification
发现潜在的内存分配器冲突问题
根据代码分析,发现以下问题:
crates/mako/src/main.rs
和crates/binding/src/lib.rs
都使用了extern crate swc_malloc
- 但是没有找到全局分配器的配置(
#[global_allocator]
) - 同时引入
swc_malloc
和tikv-jemallocator
可能会导致内存分配冲突
建议:
- 选择其中一个内存分配器作为全局分配器
- 如果需要使用
tikv-jemallocator
,应该移除swc_malloc
的依赖 - 在主入口文件中正确配置全局分配器
🔗 Analysis chain
请验证内存分配器的兼容性
当前工作区同时包含了 swc_malloc
和 tikv-jemallocator
两个内存分配器。请确认:
- 这两个分配器的共存是否会产生冲突
- 是否正确配置了全局分配器
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索全局分配器的配置
rg -l "#\[global_allocator\]"
# 检查是否有其他内存分配器相关的配置
ast-grep --pattern 'use $_malloc'
Length of output: 67
Script:
#!/bin/bash
# 搜索全局分配器的配置,包括可能的变体形式
rg -l "global_allocator|GlobalAlloc"
# 搜索 jemalloc 相关的导入和使用
rg -l "jemalloc|tikv_jemallocator"
# 搜索 swc_malloc 相关的导入和使用
rg -l "swc_malloc"
# 检查 lib.rs 或 main.rs 中的内存分配器配置
fd -e rs "(lib|main).rs$" -x cat {}
Length of output: 17159
#[derive(Deserialize, Serialize, Debug, Clone)] | ||
pub struct RustPlugin { | ||
pub path: String, | ||
pub options: 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
建议改进 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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}`); | |
} | |
} |
@@ -235,6 +235,7 @@ export interface BuildParams { | |||
}; | |||
experimental?: { | |||
webpackSyntaxValidate?: string[]; | |||
rustPlugins?: Array<[string, any]>; |
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.
💡 Codebase verification
需要在文档中添加 rustPlugins 的说明
根据检查结果,rustPlugins 已经:
- 在默认配置文件中有定义且默认值为空数组
- 作为实验性功能被正确放置在 experimental 配置下
- 在代码中有相应的类型定义和实现
但是在文档中缺少对这个实验性功能的说明,建议:
- 在 docs/config.md 和 docs/config.zh-CN.md 中添加 experimental.rustPlugins 的配置说明
- 说明其用途、类型定义和使用示例
🔗 Analysis chain
验证实验性功能的稳定性
由于这是一个实验性功能,需要确保:
- 是否需要添加功能开关或版本检查
- 是否需要在文档中说明该功能的实验性质
Let me search for more specific information about Rust plugins.
让我再检查一下 experimental 配置的相关文档。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有相关的实验性功能文档
rg -g "*.md" -i "experimental|rust.*plugin"
Length of output: 2013
Script:
#!/bin/bash
# 检查源代码中关于 rustPlugins 的使用和定义
rg -g "!*.d.ts" "rustPlugins" -A 3
# 检查是否在其他地方有相关的实验性功能标记
rg "experimental.*rust" -A 2
Length of output: 2088
Script:
#!/bin/bash
# 检查 rustPlugins 的默认配置和文档
rg -g "*.{md,json}" "experimental" -B 2 -A 5
Length of output: 13735
} | ||
|
||
fn handle_local(_attr: TokenStream2, item: TokenStream2) -> TokenStream2 { | ||
let ast = parse2::<ItemImpl>(item).unwrap(); |
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
避免使用 unwrap()
来处理错误
使用 unwrap()
在解析失败时会导致编译器 panic。建议使用 expect()
并提供有意义的错误信息,或者以更稳健的方式处理错误。
应用以下代码修改:
- let ast = parse2::<ItemImpl>(item).unwrap();
+ let ast = parse2::<ItemImpl>(item).expect("Failed to parse ItemImpl");
📝 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.
let ast = parse2::<ItemImpl>(item).unwrap(); | |
let ast = parse2::<ItemImpl>(item).expect("Failed to parse ItemImpl"); |
#[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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
缺少对 Plugin
trait 的引用
代码中使用了 dyn Plugin
,但当前作用域中未引入 Plugin
trait,可能会导致编译错误。请考虑在文件顶部添加对 Plugin
trait` 的引用。
应用以下代码修改:
+ use mako::plugin::Plugin;
Committable suggestion skipped: line range outside the PR's diff.
support rust native plugin
Summary by CodeRabbit
新功能
rustPlugins
配置选项,允许用户在构建过程中集成 Rust 插件。rustPluginResolver
,用于解析和加载 Rust 插件。文档
mako.config.default.json
和 TypeScript 类型定义,以支持新的 Rust 插件配置。样式