refactor: napi threadsafe function

[xusd320]

A simpler and more concise wrapper for napi threadsafe function.

Summary by CodeRabbit

• 新功能
  • 引入了新的 LoadResult 接口，包含 content 和 type 属性。
  • 新增 WriteFile 方法，允许用户写入文件。
• 改进
  • 更新了 build 函数的声明，增强了类型定义。
  • 优化了 JsPlugin 的方法调用，简化了线程安全函数的处理。
  • 更新了依赖项，提升了整体性能和稳定性。

[coderabbitai]

Walkthrough

本次更改涉及对多个文件的更新，主要集中在 Cargo.toml 文件的依赖项版本升级、JsPlugin 结构中的方法同步调用，以及 ThreadsafeFunction 的泛型实现改进。此外，新增了用于处理 JavaScript 钩子的结构体和方法，简化了异步处理的逻辑。整体上，这些更改旨在提高代码的可读性和可维护性。

Changes

文件路径	更改摘要
crates/binding/Cargo.toml	更新了多个依赖项的版本，包括 napi、napi-derive，移除了 rayon，并添加了新依赖 oneshot 和 serde。
crates/binding/src/js_plugin.rs	移除了 mpsc 通道，简化了 JsPlugin 结构中多个方法的实现，改为直接调用钩子并同步处理结果。
crates/binding/src/js_hook.rs	新增了 JsHooks、TsFnHooks、WriteFile 和 LoadResult 结构，定义了处理 JavaScript 钩子的功能。
crates/binding/src/lib.rs	将 rayon::spawn 替换为 thread_pool::spawn，修改了线程机制的实现。
crates/binding/src/threadsafe_function.rs	将 ThreadsafeFunction 结构改为支持两个类型参数的泛型版本，简化了异步返回值的处理，并添加了 FromNapiValue 特征实现。
packages/mako/binding.d.ts	新增 LoadResult 接口，修改 build 函数的声明，并新增 WriteFile 类。
packages/mako/binding.js	从 nativeBinding 对象中新增导出 WriteFile 方法。

Possibly related PRs

• feat(experimental): dev simple speed up #1186: 此 PR 涉及对 Cargo.toml 依赖项的更新，可能与主 PR 的整体项目配置和依赖管理相关。
• feat: generate_end with stats #1450: 此 PR 涉及 generate_end 函数的修改，可能与主 PR 中的钩子更新相关。
• perf: use hashlink, speed up generate stage 100% when building big project #1460: 此 PR 引入的新依赖可能增强性能，可能与主 PR 中的构建过程改进有关。
• feat: support progress #1466: 此 PR 中的进度报告功能可能与主 PR 的改进相关。
• feat: add bundle for windows  #1575: 此 PR 中的 Windows 支持更改可能与主 PR 的更新相关。
• fix: merge mako config #1578: 此 PR 涉及配置合并，可能与主 PR 中的更新相关。
• feat(tree-shaking): optimize import namespace used all exports to partial used of source modules #1584: 此 PR 涉及导入命名空间的优化，可能与主 PR 中的构建过程效率改进相关。
• revert: import namespace optimize  #1606: 此 PR 撤销了与导入命名空间优化相关的更改，连接到主 PR 中对导入和导出处理的优化。

Poem

🐇 在代码的草地上跳跃，
依赖更新如春风拂面，
方法同步如流水潺潺，
兔子欢呼，乐声不断，
代码更清晰，未来更灿烂！ 🌼

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between b6f4486 and b2d87fe.

Files selected for processing (2)

• crates/binding/src/js_hook.rs (1 hunks)
• packages/mako/binding.d.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• crates/binding/src/js_hook.rs
• packages/mako/binding.d.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.93%. Comparing base (9ebbe19) to head (b2d87fe).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1608   +/-   ##
=======================================
  Coverage   61.93%   61.93%           
=======================================
  Files         128      128           
  Lines       15405    15405           
=======================================
  Hits         9541     9541           
  Misses       5864     5864           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[coderabbitai]

Actionable comments posted: 5

Outside diff range and nitpick comments (7)

crates/binding/src/js_plugin.rs (4)

20-23: build_start 方法的简化实现是一个很好的改进。

新的实现直接调用钩子函数，消除了之前使用通道的复杂性。这种方法更加简洁和直接。

建议：考虑添加一个简短的注释来解释为什么传递一个空元组作为参数。这可能会帮助其他开发者更好地理解这个调用的目的。

考虑添加如下注释：

// 传递空元组作为参数，因为 build_start 钩子不需要任何特定的输入
hook.call(())?

---

Line range hint 27-45: load 方法的重构显著提高了代码的可读性和直观性。

新的实现直接调用钩子函数并处理其结果，这种方法更加简洁和高效。对不同内容类型的处理逻辑清晰，错误处理也有所改进。

建议：考虑使用 match 语句来替代多个 if let 和 return，这可能会使代码结构更加清晰。

考虑如下重构：

fn load(&self, param: &PluginLoadParam, _context: &Arc<Context>) -> Result<Option<Content>> {
    match &self.hooks.load {
        Some(hook) => {
            match hook.call(param.file.path.to_string_lossy().to_string())? {
                Some(x) => match x.content_type.as_str() {
                    "js" | "ts" => Ok(Some(Content::Js(JsContent {
                        content: x.content,
                        is_jsx: false,
                    }))),
                    "jsx" | "tsx" => Ok(Some(Content::Js(JsContent {
                        content: x.content,
                        is_jsx: true,
                    }))),
                    "css" => Ok(Some(Content::Css(x.content))),
                    _ => Err(anyhow!("Unsupported content type: {}", x.content_type)),
                },
                None => Ok(None),
            }
        },
        None => Ok(None),
    }
}

这种结构可能更容易理解和维护。

---

52-55: generate_end 方法的简化实现是一个很好的改进。

新的实现直接调用钩子函数，消除了之前使用通道的复杂性。这种方法更加简洁和直接。

建议：考虑使用 ? 运算符来简化错误处理，这可能会使代码更加简洁。

考虑如下重构：

fn generate_end(&self, param: &PluginGenerateEndParams, _context: &Arc<Context>) -> Result<()> {
    if let Some(hook) = &self.hooks.generate_end {
        hook.call(serde_json::to_value(param)?)?;
    }
    Ok(())
}

这种写法可以减少一层嵌套，使代码更加扁平化。

---

59-64: before_write_fs 方法的重构提高了代码的清晰度。

新的实现直接调用钩子函数，消除了之前使用通道的复杂性。使用 WriteFile 结构体来传递数据是一个很好的做法，它使得参数的结构更加清晰。

建议：考虑将 WriteFile 结构的创建提取到一个单独的语句中，这可能会提高代码的可读性。

考虑如下重构：

fn before_write_fs(&self, path: &std::path::Path, content: &[u8]) -> Result<()> {
    if let Some(hook) = &self.hooks._on_generate_file {
        let write_file = WriteFile {
            path: path.to_string_lossy().to_string(),
            content: content.to_vec(),
        };
        hook.call(write_file)?;
    }
    Ok(())
}

这种结构可能更容易理解，特别是当 WriteFile 结构变得更复杂时。

packages/mako/binding.d.ts (2)

55-58: 新接口 LoadResult 看起来不错，但可以考虑进一步改进。

新添加的 LoadResult 接口很好地定义了加载结果的结构。然而，为了保持一致性，可以考虑将 contentType 的类型从 string 更改为更具体的类型，例如 'css' | 'js' | string。这样可以与 JsHooks 接口中 load 函数的返回类型保持一致，同时还允许其他类型的内容。

建议修改如下：

export interface LoadResult {
  content: string;
  contentType: 'css' | 'js' | string;
}

---

231-234: 新添加的 WriteFile 类看起来不错，但可以考虑进一步完善。

WriteFile 类很好地封装了要写入的文件的路径和内容。使用 Array<number> 表示内容是处理二进制数据的好方法。然而，有几点建议可以使这个类更加健壮和易用：

1. 考虑添加一个构造函数，以便在创建实例时初始化属性。
2. 可以添加一些方法，如 getSize() 来获取内容大小，或 toBuffer() 来将内容转换为 Buffer。
3. 为了提高类型安全性，可以使用 Uint8Array 代替 Array<number>。

建议修改如下：

export class WriteFile {
  constructor(public path: string, public content: Uint8Array) {}

  getSize(): number {
    return this.content.length;
  }

  toBuffer(): Buffer {
    return Buffer.from(this.content);
  }
}

这些改进将使 WriteFile 类更加功能完善和类型安全。

packages/mako/binding.js (1)

Line range hint 1-311: 总体评估

这次更改主要涉及新增 WriteFile 方法的导出，这可能表明native binding中新增了文件写入相关的功能。虽然改动较小，但可能对项目的功能有重要影响。

建议：

1. 确保 WriteFile 方法在native binding中已正确实现。
2. 更新相关文档，包括API参考和使用示例。
3. 考虑添加集成测试，以验证 WriteFile 方法在不同平台上的行为一致性。

考虑以下建议：

1. 评估 WriteFile 方法对项目整体架构的影响，确保它符合项目的设计原则。
2. 如果 WriteFile 涉及文件系统操作，请确保考虑了权限、安全性和错误处理等方面。
3. 考虑是否需要为 WriteFile 方法添加日志记录或性能监控，以便于后续的维护和优化。

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 68de8e2 and 8060fba.

Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

Files selected for processing (8)

• crates/binding/Cargo.toml (1 hunks)
• crates/binding/src/js_plugin.rs (3 hunks)
• crates/binding/src/lib.rs (2 hunks)
• crates/binding/src/threadsafe_function.rs (1 hunks)
• crates/binding/src/tsfn.rs (3 hunks)
• crates/mako/src/utils.rs (1 hunks)
• packages/mako/binding.d.ts (2 hunks)
• packages/mako/binding.js (1 hunks)

Additional comments not posted (21)

crates/binding/Cargo.toml (5)

14-14: napi-derive 依赖版本更新

napi-derive 依赖已从 2.14.0 更新到 2.16.12。

请确保：

1. 此版本与更新后的 napi crate 版本兼容。
2. 检查项目中使用的所有 napi 相关的派生宏，确保它们在新版本中仍然有效。

建议查看 napi-derive crate 的更新日志，了解版本 2.14.0 到 2.16.12 之间的重要变更。

---

17-17: 新增 serde 依赖

项目新增了 serde 依赖，使用工作空间配置。

Serde 是 Rust 的序列化和反序列化框架。请确保：

1. 此依赖的引入是必要的，符合项目的数据处理需求。
2. 检查项目中是否已经开始使用 serde，以及使用方式是否正确。
3. 确认工作空间根目录的 Cargo.toml 中已正确定义了 serde 的版本。

#!/bin/bash
# 检查项目中是否有使用 serde 的代码
rg --type rust 'serde::|#\[derive\(.*Serialize.*\)|#\[derive\(.*Deserialize.*\)'

---

15-15: 新增 oneshot 依赖

项目新增了 oneshot 0.1.8 依赖。

oneshot crate 提供了一次性通道实现，通常用于线程间的单次通信。请确保：

1. 此依赖的引入是有意为之，符合项目的并发模型或线程间通信策略的调整。
2. 检查项目中是否已经开始使用 oneshot 通道，以及使用方式是否正确。

#!/bin/bash
# 检查项目中是否有使用 oneshot 的代码
rg --type rust 'oneshot::'

---

16-16: rayon 依赖版本更新

rayon 依赖已从 1.7.0 更新到 1.10.0。

Rayon 是 Rust 的数据并行处理库，此重大版本更新可能引入了性能改进或新功能。请确保：

1. 检查项目中使用 rayon 的代码，确保与新版本兼容。
2. 考虑利用新版本可能带来的性能改进或新特性。

建议查看 rayon crate 的更新日志，了解版本 1.7.0 到 1.10.0 之间的重要变更。

#!/bin/bash
# 检查项目中使用 rayon 的代码
rg --type rust 'rayon::'

---

13-13: napi 依赖版本和特性更新需要注意

napi 依赖已从 2.14.0 更新到 2.16.11，并且特性列表有所变化：

• 添加了 "async" 特性，可能改进了异步操作支持
• 移除了 "napi5" 特性，可能影响与 Node.js v14 的兼容性
• 移除了 "tokio_rt" 特性，可能改变了运行时处理方式

请确保这些更改与项目的 Node.js 版本要求和异步处理策略相符。同时，建议查看 napi crate 的更新日志，了解版本 2.14.0 到 2.16.11 之间的重要变更。

Verification successful

napi 依赖的更新已确认无影响

napi 依赖已更新至 2.16.11，移除的特性 "napi5" 和 "tokio_rt" 在项目中未被使用。

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查项目中是否有使用被移除的特性的代码
rg --type rust 'napi5|tokio_rt'

Length of output: 6630

---

Script:

#!/bin/bash
# 检查项目中是否有使用被移除的特性 `napi5` 或 `tokio_rt`
rg --type rust 'napi5|tokio_rt'

Length of output: 33

---

Script:

#!/bin/bash
# 检查项目中是否有使用被移除的特性 `napi5` 或 `tokio_rt`，包括所有相关文件类型
rg -i -e 'napi5' -e 'tokio_rt' --glob '*.rs' --glob 'Cargo.toml' --glob '*.toml' --glob '*.md'

Length of output: 90

crates/mako/src/utils.rs (1)

7-7: 请验证模块可见性更改的意图和影响。

将 thread_pool 模块的可见性从 pub(crate) 更改为 pub 意味着该模块现在可以从crate外部访问。这可能会带来以下影响：

1. 增加了模块的暴露范围，可能会影响代码的封装性。
2. 其他crate现在可以使用这个线程池功能，这可能是有意为之的改变。
3. 这种更改可能会影响项目的整体架构设计。

请确保这个更改是有意为之的，并且与项目的设计原则相符。同时，考虑是否需要更新相关文档来反映这一变化。

运行以下脚本来验证 thread_pool 模块的使用情况：

Verification successful

确认 thread_pool 模块的可见性更改

根据检索结果，thread_pool 模块在多个其他 crate 中被使用。将其可见性从 pub(crate) 更改为 pub 是必要的，并且不会影响代码封装性。这一更改支持了内部 crate 之间的功能共享，符合项目的设计原则。

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述：验证 thread_pool 模块的使用情况

# 测试：搜索 thread_pool 模块的使用。预期：可能会发现在其他 crate 中的新用法。
rg --type rust "use .*thread_pool" -g '!crates/mako/src/utils.rs'

Length of output: 396

crates/binding/src/js_plugin.rs (2)

1-3: 导入更改看起来合理且与重构目标一致。

新的导入反映了代码从使用 mpsc 通道转向更直接的线程安全函数调用。这种变化简化了代码结构，可能提高了性能和可读性。

• Arc 的引入确保了线程安全的引用计数。
• LoadResult、TsFnHooks 和 WriteFile 的导入表明我们现在使用这些结构来处理线程安全的函数调用。

---

Line range hint 1-64: 总体而言，这次重构显著提高了代码的可读性和简洁性。

主要改进包括：

1. 移除了 mpsc 通道，简化了线程间通信。
2. 直接调用钩子函数，使得代码流程更加直观。
3. 使用了新的数据结构（如 WriteFile），提高了参数传递的清晰度。

这些变化与 PR 的目标一致，即为 NAPI 线程安全函数提供更简单、更简洁的包装器。代码现在更容易理解和维护，可能会提高开发效率和减少潜在的错误。

建议在合并这个 PR 之前，确保进行全面的测试，特别是在多线程环境下，以确保新的实现保持了原有的线程安全性和功能正确性。

packages/mako/binding.d.ts (1)

230-230: build 函数声明的修改是正确的。

添加 declare 关键字是一个很好的改进。这明确表示该函数是在其他地方实现的，符合 TypeScript 定义文件的最佳实践。这个变更提高了类型定义的准确性和清晰度。

crates/binding/src/lib.rs (2)

14-14: 新的导入语句看起来不错。

这个新的导入语句引入了内部的线程池实现，这可能是为了替换之前使用的外部线程库。这个改变看起来是为了配合后续在 build 函数中的修改。

---

229-229: 线程池实现的更改看起来合理。

将 rayon::spawn 替换为 thread_pool::spawn 与之前添加的导入语句相符。这个改变表明项目正在从 Rayon 库转向内部的线程池实现。

建议：

1. 确保新的 thread_pool::spawn 函数与之前的 rayon::spawn 具有相似的行为和性能特征。
2. 考虑添加注释解释为什么要进行这个更改，以及新实现的任何特殊考虑。

请运行以下脚本来验证 thread_pool::spawn 的使用情况：

packages/mako/binding.js (1)

307-307: 新增 WriteFile 方法导出

这些更改引入了一个新的 WriteFile 方法导出。这与 PR 的目标一致，即重构 NAPI 线程安全函数。

请运行以下脚本以验证 WriteFile 方法在代码库中的使用情况：

建议：

1. 确保在相关文档中更新了 WriteFile 方法的用法说明。
2. 考虑添加单元测试来验证 WriteFile 方法的功能。

Also applies to: 309-309

Verification successful

WriteFile 方法导出验证通过

根据脚本结果，WriteFile 方法已在 packages/mako/binding.js 中成功导出，并在类型声明文件 binding.d.ts 中进行了声明。目前在代码库中未发现其他模块引用或使用该方法。

建议：

1. 确保在相关文档中添加 WriteFile 方法的使用说明。
2. 添加相应的单元测试以验证 WriteFile 方法的功能。

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述：验证 WriteFile 方法的使用情况

# 测试：搜索 WriteFile 方法的使用。预期：找到相关的使用或导入。
rg --type js --type ts 'WriteFile'

Length of output: 218

crates/binding/src/threadsafe_function.rs (1)

66-67: 验证手动实现的 Send 和 Sync 是否安全

在第 66-67 行，手动为 ThreadsafeFunction 实现了 Send 和 Sync。需要确保结构体中的所有字段在多线程环境下是安全的。特别地，napi_env 和 RawThreadsafeFunction 是否满足线程安全要求，需要进行仔细验证。

请运行以下脚本以检查 RawThreadsafeFunction 和 napi_env 的线程安全性：

crates/binding/src/tsfn.rs (8)

2-3: 新增依赖导入

引入了 napi::NapiRaw 和 serde_json::Value，确保在后续代码中正确使用这些新导入的模块。

---

64-67: 更新 TsFnHooks 结构体字段类型

TsFnHooks 结构体中的字段类型已更新为更简洁的泛型类型。请确保在代码的其他部分中，这些字段的使用方式已相应调整，避免类型不匹配或运行时错误。

---

79-81: 验证 build_start 的 unsafe 块使用

在初始化 build_start 时使用了 unsafe 块和 from_napi_value 方法。请确认此操作是安全的，符合 Rust 的内存安全原则，避免潜在的未定义行为。

---

83-87: 检查 generate_end 的 ThreadsafeFunction 创建

在初始化 generate_end 时，使用了泛型类型的 ThreadsafeFunction。请确保 Value 类型在此上下文中适用，并验证 unsafe 块的正确性，以确保内存安全。

---

90-92: 验证 load 的 unsafe 块使用

在创建 load 的 ThreadsafeFunction 时，使用了 unsafe 块。请确认使用 from_napi_value 方法的安全性，并确保 String 和 Option<LoadResult> 类型的正确性。

---

94-96: 检查 _on_generate_file 的 ThreadsafeFunction 初始化

在初始化 _on_generate_file 时，使用了 unsafe 块和 from_napi_value 方法。请确保新结构体 WriteFile 与此处的泛型类型匹配，并验证内存安全性。

---

102-104: 新增 WriteFile 结构体

添加了 WriteFile 结构体，包含 path: String 和 content: Vec<u8> 字段。该结构体用于文件写操作，定义清晰，类型合适。

---

108-111: 更新 LoadResult 结构体以解决关键字冲突

在 LoadResult 结构体中添加了 use_nullable = true 属性，并为 content_type 字段添加了 #[napi(js_name = "type")] 注解。这解决了 Rust 中关键字 type 与 JavaScript 字段命名冲突的问题，确保在 JavaScript 侧可以正确访问。

[coderabbitai]

Actionable comments posted: 5

Outside diff range and nitpick comments (6)

Cargo.toml (3)

8-8: 考虑使用更灵活的版本约束

对 mimalloc-rust 的版本进行精确固定（"=0.2.1"）确保了构建的一致性，但可能会阻止自动更新到包含潜在改进或错误修复的新版本。建议考虑使用更灵活的版本约束，如 "^0.2.1"，以允许兼容的小版本更新。

---

9-9: 检查 oneshot 是否有更新的版本

添加 oneshot 依赖对于实现单次使用的通道很有用。然而，指定的版本 0.1.8 相对较旧。建议检查是否有更新的稳定版本可用，以确保获得最新的功能和潜在的性能改进。

---

12-12: 考虑记录禁用默认特性的原因

将 swc_core 的版本固定为 "=0.83.22" 确保了构建的一致性，但可能会阻止自动更新到新版本。禁用默认特性允许更精细地控制所包含的功能，这可能是有意为之的。建议在代码注释或文档中说明禁用默认特性的原因，以便其他开发者理解这一决定。

crates/binding/src/threadsafe_function.rs (2)

28-36: FromNapiValue 实现是个很好的添加，但可以改进错误处理

新增的 FromNapiValue 实现使得 ThreadsafeFunction 能够更容易地与 NAPI 系统集成，这是一个很好的改进。

建议：

1. 考虑在创建 ThreadsafeFunction 时添加更多的错误检查。
2. 可以使用 ? 操作符来简化错误处理，例如：

let tsfn = Tsfn::from_napi_value(env, napi_val)?;
Ok(Self {
    tsfn,
    env,
    _phantom: PhantomData,
})

这样可以使代码更加简洁和健壮。

---

66-67: Sync 和 Send 特征实现是必要的，但需要文档说明

为 ThreadsafeFunction<T, R> 实现 Sync 和 Send 特征是正确的，这使得该结构可以在多线程环境中安全使用。然而，使用 unsafe 意味着实现者需要确保线程安全。

建议：

1. 添加注释解释为什么这些实现是安全的。
2. 说明在哪些情况下可能会违反线程安全，以及用户应该如何正确使用这个结构来保证线程安全。

例如：

// SAFETY: ThreadsafeFunction is safe to share between threads because...
unsafe impl<T: 'static, R> Sync for ThreadsafeFunction<T, R> {}

// SAFETY: ThreadsafeFunction is safe to send to another thread because...
unsafe impl<T: 'static, R> Send for ThreadsafeFunction<T, R> {}

这样的文档可以帮助其他开发者理解和正确使用这个结构。

crates/binding/src/js_hook.rs (1)

15-55: 考虑将复杂的类型定义抽取为 TypeScript 类型别名

在第15行至第55行，generate_end 的 ts_type 注解包含了复杂的类型定义。为了提高可读性和维护性，建议将该类型定义抽取为 TypeScript 类型别名，并在注解中引用。

示例：

在 TypeScript 代码中定义类型别名：

type GenerateEndData = {
  isFirstCompile: boolean;
  time: number;
  stats: {
    // ...（保留原有定义）
  };
};

然后在注解中使用该类型别名：

- #[napi(ts_type = r#"(data: { ... }) => void"#)]
+ #[napi(ts_type = "(data: GenerateEndData) => void")]

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4a6cccd and 8905148.

Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

Files selected for processing (8)

• Cargo.toml (1 hunks)
• crates/binding/Cargo.toml (1 hunks)
• crates/binding/src/js_hook.rs (1 hunks)
• crates/binding/src/js_plugin.rs (3 hunks)
• crates/binding/src/lib.rs (2 hunks)
• crates/binding/src/threadsafe_function.rs (1 hunks)
• crates/mako/Cargo.toml (1 hunks)
• packages/mako/binding.d.ts (2 hunks)

Files skipped from review as they are similar to previous changes (3)

• crates/binding/src/js_plugin.rs
• crates/binding/src/lib.rs
• packages/mako/binding.d.ts

Additional comments not posted (12)

Cargo.toml (3)

6-6: 新增依赖项 anyhow 看起来不错。

添加 anyhow 依赖是一个很好的选择。这个库提供了灵活的错误处理机制，可以简化错误管理并提高代码可读性。版本 1.0.71 是一个相对较新的稳定版本。

---

10-10: serde 依赖更新看起来很好

将 serde 更新到版本 1.0.171 并启用 "derive" 特性是一个很好的改进。这确保了项目使用最新的稳定版本，同时 "derive" 特性的启用简化了序列化和反序列化的实现。

---

6-12: 总体依赖更新看起来不错，有一些小建议

整体而言，Cargo.toml 文件的更新是积极的。新增的 anyhow 和 oneshot 依赖项增强了项目的错误处理和通信能力。serde 的更新保持了项目的最新状态。对于 mimalloc-rust 和 swc_core，建议考虑使用更灵活的版本约束，并记录特定配置选择的原因。这些小改进可以进一步提高项目的可维护性和透明度。

crates/binding/Cargo.toml (5)

10-10: 依赖项更改看起来不错！

将 anyhow 依赖项更改为使用工作空间配置是一个很好的做法。这有助于在整个项目中保持一致的版本，简化了依赖管理。

---

15-15: 新增依赖项 oneshot 看起来合理

添加 oneshot 依赖项并使用工作空间配置是个好主意。这个 crate 通常用于异步上下文中的一次性通道通信，可能与 napi 的新 "async" 特性有关。

不过，为了更好地理解这个变更：

1. 能否提供一些关于 oneshot 在项目中预期用途的更多信息？
2. 这是否与 napi 特性的变更有直接关联？

---

16-16: 新增 serde 依赖项是个好主意

添加 serde 依赖项并使用工作空间配置是不错的选择。虽然 serde_json 已经间接引入了 serde，但直接添加 serde 可能意味着项目将更广泛地使用其功能。

为了更好地理解这个变更：

1. 能否解释一下直接添加 serde 的具体原因？
2. 项目是否计划使用 serde 的一些特定功能，而这些功能不能通过 serde_json 获得？

---

10-17: 请解释移除 rayon 依赖的原因和影响

注意到 rayon 依赖项已被移除。考虑到 rayon 通常用于 Rust 中的并行计算，这个变更可能表明项目处理并行性的方法发生了变化。

请提供以下信息：

1. 移除 rayon 的具体原因是什么？
2. 这是否与添加 napi 的 "async" 特性有关？
3. 之前依赖 rayon 的功能现在是如何实现的？
4. 这个变更会对项目的性能产生什么影响？

可以运行以下脚本来检查项目中是否仍在使用 rayon：

#!/bin/bash
# 描述：检查是否仍在使用 rayon

# 测试：搜索 rayon 的使用。预期：没有匹配项或很少匹配项。
echo "Searching for rayon usage:"
rg --type rust 'rayon'

---

13-14: 依赖项版本更新良好，但需要验证功能变更

napi 和 napi-derive 的版本更新是好的，可以获得最新的安全修复和功能改进。然而，napi 的功能特性发生了变化：

1. 移除了 "napi5" 和 "tokio_rt" 特性
2. 添加了 "async" 特性

请确认这些更改不会对项目的功能产生负面影响。特别是，验证移除 "tokio_rt" 特性是否会影响项目中的异步操作。

可以运行以下脚本来检查项目中是否仍在使用已移除的特性：

crates/binding/src/threadsafe_function.rs (4)

3-11: 导入声明看起来很合理

新增的导入项目对于实现新版本的 ThreadsafeFunction 是必要的。使用 anyhow::Result 表明改进了错误处理机制，这是一个很好的做法。

---

12-15: 结构定义的改进很好，但需要澄清错误策略

新的泛型结构 ThreadsafeFunction<P, R> 提高了灵活性，这是一个很好的改进。然而，使用 ErrorStrategy::Fatal 可能需要进一步解释。

请考虑添加注释说明为什么选择 Fatal 错误策略，或者考虑是否可以使用更温和的错误处理方式。

---

18-25: Clone 实现的简化是个很好的改进

新的 Clone 实现更加简洁明了，直接利用了 Tsfn 的 Clone 实现。这种方式减少了代码复杂性，提高了可维护性。

---

1-67: 总体评价：重构改进显著，但仍有优化空间

这次对 ThreadsafeFunction 的重构带来了显著的改进：

1. 新的泛型结构提高了灵活性。
2. call 方法现在能够处理同步和异步返回。
3. 与 NAPI 系统的集成更加简便。

主要改进建议：

1. 增强错误处理，避免过度使用 unwrap() 和 expect()。
2. 为 unsafe 实现添加详细的安全性文档。
3. 考虑解释或重新评估使用 ErrorStrategy::Fatal 的决定。

总的来说，这是一个很好的重构，通过实施上述建议，可以进一步提高代码的健壮性和可维护性。

[coderabbitai]

Actionable comments posted: 1

Outside diff range and nitpick comments (1)

crates/binding/src/threadsafe_function.rs (1)

28-37: FromNapiValue 实现的改进建议

FromNapiValue 的实现对于与 N-API 进行互操作是必要的，实现方式总体上是正确的。然而，有一个小的改进建议：

impl<P: 'static + JsValuesTupleIntoVec, R> FromNapiValue for ThreadsafeFunction<P, R> {
    unsafe fn from_napi_value(env: napi_env, napi_val: napi_value) -> napi::Result<Self> {
        let tsfn = Tsfn::from_napi_value(env, napi_val)?;
        Ok(Self {
            tsfn,
            env,
            _phantom: PhantomData,
        })
    }
}

建议在创建 Self 之前，使用 map_err 来提供更具体的错误信息。这样可以在出错时提供更有用的上下文信息。

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8905148 and b6f4486.

Files selected for processing (1)

• crates/binding/src/threadsafe_function.rs (1 hunks)

Additional comments not posted (3)

crates/binding/src/threadsafe_function.rs (3)

3-11: 导入声明的改进

这些新的导入声明显示了代码在错误处理和异步操作支持方面的改进。使用 anyhow 进行错误处理是一个很好的实践，它提供了更灵活和表达性强的错误处理机制。同时，引入 spawn 和 Promise 表明代码现在能够更好地处理异步操作。

---

12-15: ThreadsafeFunction 结构体的改进

新的 ThreadsafeFunction 结构体定义有以下几个显著的改进：

1. 泛型实现（<P: 'static, R>）提高了代码的灵活性和复用性。
2. 使用 napi crate 中的 Tsfn 简化了实现，有助于代码维护。
3. PhantomData<R> 的使用正确处理了未使用的类型参数 R，避免了编译器警告。

这些改变使得 ThreadsafeFunction 更加通用和强大，能够适应更多的使用场景。

---

18-25: Clone 实现的简化

ThreadsafeFunction 的 Clone 实现非常简洁和高效。它正确地克隆了 tsfn 字段，并复制了 env 值。这种实现依赖于底层 Tsfn 类型的 Clone 实现，这是一个很好的做法，因为它避免了不必要的复杂性，并确保了一致的行为。