Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[WIP] feat: native plugin init #1691

wants to merge 4 commits into from

Conversation

Jinbao1001
Copy link
Member

@Jinbao1001 Jinbao1001 commented Nov 13, 2024

support rust native plugin

Summary by CodeRabbit

  • 新功能

    • 引入了新的 rustPlugins 配置选项,允许用户在构建过程中集成 Rust 插件。
    • 增强了构建和开发过程中的错误处理和配置验证。
    • 添加了异步函数 rustPluginResolver,用于解析和加载 Rust 插件。
  • 文档

    • 更新了 mako.config.default.json 和 TypeScript 类型定义,以支持新的 Rust 插件配置。
  • 样式

    • 修改了模块和依赖项的可见性,以提高可用性和访问性。

Copy link
Contributor

coderabbitai bot commented Nov 13, 2024

Walkthrough

本次更改主要涉及多个 Cargo.toml 文件和相关源代码的修改,重点在于依赖项的管理。移除了 mimalloc-rust 依赖并新增了 swc_malloc 依赖。此外,更新了多个文件以反映这些依赖的变化,包括对全局分配器的配置、构建参数的结构以及插件的动态加载机制。整体上,增强了插件系统的灵活性和可扩展性,同时调整了模块的可见性和配置选项。

Changes

文件路径 更改摘要
Cargo.toml 移除依赖 mimalloc-rust,新增依赖 swc_malloc 版本 1.0.0
crates/binding/Cargo.toml 移除非 Linux 目标的条件依赖 mimalloc-rusttikv-jemallocator,新增依赖 swc_malloc
crates/binding/src/lib.rs 移除全局分配器定义,新增外部 crate swc_malloc,更新 BuildParams 结构以包含新字段 rustPlugins
crates/mako/Cargo.toml 新增依赖 swc_malloc,更新 swc_core 依赖以包含新特性,移除非 Linux 条件依赖,新增多个依赖项。
crates/mako/src/ast.rs 更新 utils 模块的可见性,从 pub(crate) 改为 pub
crates/mako/src/compiler.rs 更新 Compiler::new 方法以支持动态加载 Rust 插件,修改方法签名以允许对配置的修改。
crates/mako/src/config.rs 更新 serde_json 导入,新增变量 overrides_json 以处理实验性配置,修改配置处理逻辑。
crates/mako/src/config/experimental.rs 新增结构体 RustPlugin,并在 ExperimentalConfig 中新增字段 rust_plugins
crates/mako/src/config/mako.config.default.json experimental 部分新增属性 "rustPlugins": []
crates/mako/src/lib.rs module 模块改为公有,新增 swc_coreswc_malloc 的公有导出。
crates/mako/src/main.rs 移除全局分配器配置,保留主函数逻辑不变。
crates/mako_plugin_macro/Cargo.toml 新增 mako_plugin_macro 包配置,指定版本和依赖项。
crates/mako_plugin_macro/src/lib.rs 新增 mako_pluginwith_local 宏,处理插件创建和上下文管理。
packages/bundler-mako/index.js 更新构建和开发过程中的错误处理,新增 rustPlugins 属性到 makoConfig 对象。
packages/mako/binding.d.ts BuildParams 接口中新增属性 rustPlugins
packages/mako/src/binding.d.ts BuildParams 接口中新增属性 rustPlugins
packages/mako/src/index.ts 新增对 rustPluginResolver 的导入,并在构建函数中处理 Rust 插件。
packages/mako/src/rustPlugins/index.ts 新增异步函数 rustPluginResolver,用于解析插件列表并返回路径和选项。

Possibly related PRs

Suggested reviewers

  • sorrycc
  • stormslowly

🐰 在草地上跳跃,代码如花绽放,
新增插件,旧依赖已远航。
配置灵活,构建更顺畅,
小兔子欢呼,代码更辉煌! 🌼

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 40daa0d and 5cde533.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Walkthrough

This 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 swc_malloc for better performance and compatibility.

Changes

Files Summary
Cargo.lock, Cargo.toml Added dependencies for Rust plugins and updated memory allocator to swc_malloc. Removed mimalloc-rust and tikv-jemallocator.
crates/binding/src/lib.rs, crates/mako/src/main.rs Removed global allocator settings and added swc_malloc as the allocator.
crates/mako/src/compiler.rs Integrated loading of Rust plugins using libloading.
crates/mako/src/config.rs, crates/mako/src/config/experimental.rs Added support for experimental Rust plugin configuration.
crates/mako_plugin_macro/src/lib.rs Introduced macros for Rust plugin creation and handling.
packages/mako/src/index.ts, packages/mako/src/rustPlugins/index.ts Added logic to resolve and load Rust plugins in the JavaScript environment.
packages/bundler-mako/index.js, packages/mako/binding.d.ts, packages/mako/src/binding.d.ts Updated TypeScript definitions and configuration to support Rust plugins.

@@ -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 {

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();

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.

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 48.14815% with 14 lines in your changes missing coverage. Please review.

Project coverage is 55.24%. Comparing base (40daa0d) to head (5cde533).

Files with missing lines Patch % Lines
crates/mako/src/config.rs 50.00% 7 Missing ⚠️
crates/mako/src/compiler.rs 55.55% 4 Missing ⚠️
crates/mako/src/config/experimental.rs 25.00% 3 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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: 建议完善类型定义和文档说明

考虑以下几点改进建议:

  1. any 类型过于宽松,建议定义具体的插件配置接口以提供更好的类型安全性和开发体验
  2. 缺少相关文档说明,建议添加 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 可能过于宽松。建议:

  1. 为插件配置创建一个专门的接口,以提供更好的类型安全性和开发体验
  2. 考虑添加相关的类型文档,说明每个配置项的用途

示例改进:

interface RustPluginConfig {
  // 根据实际需求定义配置项
  enabled?: boolean;
  options?: Record<string, unknown>;
}

// 然后更新类型定义
rustPlugins?: Array<[string, RustPluginConfig]>;
crates/mako/src/config.rs (1)

235-255: 建议增强错误处理机制

当前实现在处理实验性配置时可以更加健壮:

  1. 解析 JSON 时的错误处理可以更详细
  2. 建议添加对实验性配置内容的验证

建议按如下方式改进:

 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

📥 Commits

Reviewing files that changed from the base of the PR and between 40daa0d and 5cde533.

⛔ 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.rsplugins/*)都是 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 是一个重要变更。建议:

  1. 在不同操作系统和架构上进行充分测试
  2. 评估对插件性能的潜在影响
  3. 确保内存分配策略的改变不会影响插件的稳定性

运行以下脚本检查项目中的目标平台支持:

✅ Verification successful

Let's analyze the workspace configuration and platform-specific code to better understand the memory allocator usage and platform support.


swc_malloc 的使用是安全且合适的

基于代码分析结果:

  1. swc_malloc 作为工作空间依赖被正确配置,版本为 1.0.0
  2. 项目中只有少量平台特定代码(仅针对 Windows 路径处理),说明内存分配器的改变不会影响跨平台兼容性
  3. swc_malloc 与 swc_core 一起被使用,这是标准的 SWC 工具链组合
  4. 内存分配器的变更仅限于必要的系统级组件
🏁 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: ⚠️ Potential issue

验证插件路径的有效性

在将插件路径添加到结果数组之前,建议验证 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_coreswc_malloc 可能会:

  1. 暴露过多实现细节
  2. 增加 API 的不稳定性
  3. 使版本管理变得复杂

建议:

  • 考虑封装必要的功能而不是直接重导出
  • 添加文档说明重导出的原因和使用场景

13-13: 请确认模块可见性更改的必要性

module 模块改为公开可能会破坏封装性。建议:

  1. 确认是否所有内容都需要公开
  2. 考虑只公开必要的子模块或接口
  3. 添加文档注释说明公开的原因和用途
✅ Verification successful

将 module 模块改为公开是必要的

从代码分析来看,module 模块确实需要公开,因为:

  1. 它包含了许多核心类型如 ModuleIdDependencyModuleAst 等,这些类型被整个项目广泛使用
  2. 这些类型在插件系统和编译流程中都是必需的
  3. 代码中有大量合法的跨模块引用,表明这是有意为之的架构设计

建议:

  • 为 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 原生插件的入口文件,建议考虑以下几点:

  1. 插件的生命周期管理
  2. 插件加载失败的错误处理机制
  3. 插件系统的初始化时机

需要我协助设计插件系统的架构文档或实现相关代码吗?


4-5: 验证内存分配器更改的影响

引入 swc_malloc 作为新的内存分配器替代了之前的平台特定配置。这个改动简化了内存管理策略,但需要确保在所有目标平台上都能正常工作。

✅ Verification successful

内存分配器配置变更已验证

引入 swc_malloc 作为外部依赖是合理的,因为:

  1. 项目同时依赖了 swc_malloctikv-jemallocator 作为内存分配器
  2. 这些依赖在 Cargo.toml 中正确配置
  3. 在 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_transformswc_common 特性是支持原生插件所必需的,但需要确保版本兼容性。

建议:

  1. 确保这些新特性与现有的转换流程兼容
  2. 考虑添加相关的集成测试来验证插件系统的正确性

87-87: 检查插件系统相关依赖的完整性

新增的依赖(libloadingproc-macro2quotesyn)构成了插件系统的基础架构。

建议:

  1. libloading 用于动态加载插件,建议添加错误处理机制
  2. 宏相关的依赖(proc-macro2quotesyn)建议考虑是否需要在特性标志后面
  3. 考虑添加插件系统的文档说明

运行以下脚本检查依赖使用情况:

#!/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/makocrates/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

版本固定是合理的做法

从代码库分析来看,这个版本固定是合理的:

  1. 项目中同时固定了其他重要依赖的版本,如 oxc_resolver = "=1.7.0",并且在注释中说明了具体原因
  2. swc_emotion 作为 SWC 生态系统的一部分,与其他 SWC 相关依赖(如 swc_coreswc_error_reportersswc_node_comments)版本需要保持兼容
  3. 这种精确的版本控制有助于确保构建的稳定性和可重现性
🏁 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 插件与现有插件系统的集成

需要确认以下几点:

  1. Rust 插件是否需要在 watch 模式下进行特殊清理
  2. 是否需要在插件加载流程中添加特殊处理

运行以下脚本来验证插件系统的集成:

✅ 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 插件系统已经具备完整的集成机制:

  1. 在 watch 模式下,插件系统通过 watch_changes 钩子函数提供了标准的文件变更处理机制
  2. Rust 插件在编译器初始化时会被正确加载(见 crates/mako/src/compiler.rs
  3. 插件系统提供了统一的清理机制,不需要特殊处理

无需在插件加载流程中添加额外的特殊处理。现有的插件系统架构已经完全支持 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 的类型定义过于宽松:

  1. any 类型不够严格,建议定义具体的插件配置接口
  2. 缺少相关文档说明插件的使用方式
  3. 作为实验性功能,建议添加相关警告注释

建议修改为更严格的类型定义:

-        rustPlugins?: Array<[string, any]>;
+        /**
+         * 实验性功能:Rust 原生插件支持
+         * @experimental 该功能仍在开发中,接口可能发生变化
+         */
+        rustPlugins?: Array<[string, RustPluginConfig]>;

另外需要在相应位置添加 RustPluginConfig 接口定义。

#!/bin/bash
# 检查是否已实现插件加载逻辑
rg "rustPlugins?" --type rust

3-4: 验证内存分配器的变更影响

引入 swc_malloc 作为新的内存分配器需要确保:

  1. 系统性能不会受到负面影响
  2. 与现有的内存管理机制兼容
  3. 在不同平台上都能正常工作
✅ 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

配置覆盖行为已正确实现

从代码分析结果来看,配置源的加载顺序是正确的:

  1. 默认配置(DEFAULT_CONFIG)
  2. 配置文件(abs_config_file)
  3. 命令行配置(cli_config)
  4. 覆盖配置(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 插件提供了扩展点。

Comment on lines +4 to +11
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)]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

需要添加错误处理机制

当前实现缺少错误处理逻辑。插件解析和导入过程中可能会发生错误,建议添加 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.

Suggested change
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}`);
}
}

Comment on lines +14 to 15
swc_malloc = "1.0.0"
tikv-jemallocator = { version = "=0.5.4", features = ["disable_initial_exec_tls"] }
Copy link
Contributor

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.rscrates/binding/src/lib.rs 都使用了 extern crate swc_malloc
  • 但是没有找到全局分配器的配置(#[global_allocator]
  • 同时引入 swc_malloctikv-jemallocator 可能会导致内存分配冲突

建议:

  • 选择其中一个内存分配器作为全局分配器
  • 如果需要使用 tikv-jemallocator,应该移除 swc_malloc 的依赖
  • 在主入口文件中正确配置全局分配器
🔗 Analysis chain

请验证内存分配器的兼容性

当前工作区同时包含了 swc_malloctikv-jemallocator 两个内存分配器。请确认:

  1. 这两个分配器的共存是否会产生冲突
  2. 是否正确配置了全局分配器
🏁 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

Comment on lines +5 to +9
#[derive(Deserialize, Serialize, Debug, Clone)]
pub struct RustPlugin {
pub path: String,
pub options: String,
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

建议改进 RustPlugin 结构体的设计

当前实现存在以下需要改进的地方:

  1. 缺少结构体和字段的文档注释,建议添加以说明用途和预期值
  2. 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.

Suggested change
#[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,
}

Comment on lines +47 to +51
const rustPlugins = params.config.experimental?.rustPlugins;
if (rustPlugins) {
params.config.experimental!.rustPlugins =
await rustPluginResolver(rustPlugins);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

建议增强错误处理和类型安全性

当前实现有以下几点需要注意:

  1. 缺少对 rustPluginResolver 执行失败的错误处理
  2. 直接修改 params.config.experimental 可能导致类型不安全
  3. 建议添加日志记录以便于调试

建议按照以下方式重构:

  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.

Suggested change
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]>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

需要在文档中添加 rustPlugins 的说明

根据检查结果,rustPlugins 已经:

  1. 在默认配置文件中有定义且默认值为空数组
  2. 作为实验性功能被正确放置在 experimental 配置下
  3. 在代码中有相应的类型定义和实现

但是在文档中缺少对这个实验性功能的说明,建议:

  • 在 docs/config.md 和 docs/config.zh-CN.md 中添加 experimental.rustPlugins 的配置说明
  • 说明其用途、类型定义和使用示例
🔗 Analysis chain

验证实验性功能的稳定性

由于这是一个实验性功能,需要确保:

  1. 是否需要添加功能开关或版本检查
  2. 是否需要在文档中说明该功能的实验性质

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();
Copy link
Contributor

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.

Suggested change
let ast = parse2::<ItemImpl>(item).unwrap();
let ast = parse2::<ItemImpl>(item).expect("Failed to parse ItemImpl");

Comment on lines +15 to +18
#[no_mangle]
pub fn _plugin_create(option: std::string::String) -> std::sync::Arc<dyn Plugin> {
std::sync::Arc::new(#struct_name::new(option))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

缺少对 Plugin trait 的引用

代码中使用了 dyn Plugin,但当前作用域中未引入 Plugin trait,可能会导致编译错误。请考虑在文件顶部添加对 Plugin trait` 的引用。

应用以下代码修改:

+ use mako::plugin::Plugin;

Committable suggestion skipped: line range outside the PR's diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant