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

feat: add resolve_id plugin hook #1625

Merged
merged 3 commits into from
Oct 10, 2024
Merged

feat: add resolve_id plugin hook #1625

merged 3 commits into from
Oct 10, 2024

Conversation

sorrycc
Copy link
Member

@sorrycc sorrycc commented Oct 9, 2024

part of #1238

Summary by CodeRabbit

  • 新功能

    • JsHooksTsFnHooks 结构中添加了可选字段 resolve_id,支持基于源和导入者字符串解析标识符。
    • 新增 Resolution 结构体,封装文件路径及其相关信息。
    • resolve_id 方法被引入到 JsPlugin 结构,允许插件解析标识符。
    • Plugin 接口中新增 resolve_id 方法,增强插件功能。
    • resolve 模块的可见性更新为公共,便于外部访问。
  • 文档

    • 更新了 binding.d.ts 文件,增加了 JsHooks 接口的可选方法 resolveId 及新接口 ResolveIdResult
    • 更新了 Mako 配置文档,增加了新的配置项和示例,提升了可读性和易用性。
  • 测试

    • 在测试用例中增加了对 resolve_id 钩子的验证。

Copy link
Contributor

coderabbitai bot commented Oct 9, 2024

Walkthrough

此拉取请求引入了对多个结构体的修改,主要涉及 JsHooksTsFnHooks 结构体,新增了可选字段 resolve_id。同时,JsPlugin 结构体增加了 resolve_id 方法,允许根据源和导入者字符串解析标识符。此外,resolve 模块的可见性被修改为公共,新的 Resolution 结构体被引入以封装文件路径相关信息,并且多个相关方法和测试也进行了更新。

Changes

文件路径 更改摘要
crates/binding/src/js_hook.rs 新增字段 pub resolve_id: Option<JsFunction>pub resolve_id: Option<ThreadsafeFunction<(String, String), Option<ResolveIdResult>>>。新增结构体 ResolveIdResult
crates/binding/src/js_plugin.rs 新增方法 fn resolve_id(&self, source: &str, importer: &str, _context: &Arc<Context>) -> Result<Option<ResolverResource>>
crates/mako/src/lib.rs resolve 模块的可见性从私有更改为公共。
crates/mako/src/plugin.rs 新增方法 fn resolve_id(&self, _source: &str, _importer: &str, _context: &Arc<Context>) -> Result<Option<ResolverResource>>
crates/mako/src/resolve.rs 新增模块 resolution,更新 ResolvedResource 使用新的 Resolution 结构体,修改 resolvedo_resolve 函数。
crates/mako/src/resolve/resolution.rs 新增结构体 pub struct Resolution,包含路径、查询、片段和 PackageJson 的可选引用。
crates/mako/src/resolve/resource.rs 修改 Resolution 的导入语句,从外部 crate 更改为本地模块。
e2e/fixtures/plugins/expect.js 增加新的断言以验证 resolve_id 钩子的功能。
e2e/fixtures/plugins/plugins.config.js 新增异步方法 async resolveId(source, importer)
e2e/fixtures/plugins/src/index.tsx 新增控制台日志语句以要求模块 resolve_id
packages/mako/binding.d.ts 扩展 JsHooks 接口,新增可选方法 resolveId 和新接口 ResolveIdResult

Possibly related PRs

Suggested reviewers

  • stormslowly

Poem

🐰 在草地上跳跃,欢声笑语,
新增的 resolve_id,如星星闪烁。
结构体更新,功能更强,
让我们一起庆祝,代码更辉煌!
兔子们欢呼,代码如歌,
这是我们的时刻,永远不落。 🌼


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
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: 2

🧹 Outside diff range and nitpick comments (11)
e2e/fixtures/plugins/expect.js (1)

11-13: 新断言看起来不错!

新添加的断言正确地验证了 resolve_id 钩子的功能。这与 PR 的目标一致。

建议添加一个简短的注释来解释这个新断言的目的。例如:

 // resolve_id hook
+// 验证 resolve_id 钩子是否正确实现
 assert(content.includes(`resolve_id mocked`), `resolve_id hook works`);

这将有助于其他开发人员更好地理解测试的意图。

e2e/fixtures/plugins/plugins.config.js (1)

24-30: 新的 resolveId 方法实现看起来不错,但有一个小建议。

这个新方法很好地实现了 resolve_id 插件钩子的功能。它正确地处理了特定的模块解析,并为其他情况返回 null。

不过,有一个小建议:

考虑在生产环境中移除或条件化 console.log 语句。您可以使用环境变量来控制日志输出,例如:

 async resolveId(source, importer) {
   if (source === 'resolve_id') {
-    console.log('resolveId', source, importer);
+    if (process.env.NODE_ENV !== 'production') {
+      console.log('resolveId', source, importer);
+    }
     return { id: require('path').join(__dirname, 'resolve_id_mock.js') };
   }
   return null;
 }

这样可以在开发时保留有用的日志,同时避免在生产环境中输出不必要的信息。

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

Line range hint 1-56: 文件总结

此更改将 resolve 模块的可见性从私有改为公共,这是实现新的 resolve_id 插件钩子的必要步骤。这个修改与 PR 的目标一致,并且可能会影响其他模块对 resolve 模块的访问。请确保这个更改不会破坏现有的模块封装或导致意外的副作用。

建议在实现新功能时,仔细考虑模块的可见性,以确保正确的封装和最小特权原则。

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

54-54: 新增的 resolveId 方法符合预期,建议添加注释

新增的 resolveId 方法符合 PR 的目标,即添加 resolve_id 插件钩子。方法签名看起来合适,返回类型与新添加的 ResolveIdResult 接口匹配。

为了提高代码的可读性和可维护性,建议为该方法添加简短的 JSDoc 注释,解释其用途和参数含义。例如:

/**
 * 解析给定源和导入者的标识符
 * @param source 要解析的源
 * @param importer 导入者的上下文
 * @returns 包含解析后标识符的 Promise
 */
resolveId?: (source: string, importer: string) => Promise<{ id: string }>;

64-66: 新增的 ResolveIdResult 接口结构合理,建议添加注释

新增的 ResolveIdResult 接口结构简单明了,与 JsHooksresolveId 方法的返回类型相匹配。这种设计有助于提高代码的类型安全性和可读性。

为了进一步提高代码的可读性和可维护性,建议为该接口添加简短的 JSDoc 注释,解释其用途。例如:

/**
 * 表示解析标识符的结果
 */
export interface ResolveIdResult {
  /**
   * 解析后的标识符
   */
  id: string;
}
crates/mako/src/resolve/resolution.rs (2)

16-16: 修改重复的文档注释以准确描述方法功能

path方法和into_path_buf方法的文档注释都是“返回不包含查询和片段的路径”,但两者的行为不同。建议修改注释,以清晰区分两者的功能。

修改建议:

 /// 返回不包含查询和片段的路径
 pub fn path(&self) -> &Path {
     &self.path
 }

- /// 返回不包含查询和片段的路径
+ /// 消耗自身,返回路径的所有权,不包含查询和片段
 pub fn into_path_buf(self) -> PathBuf {
     self.path
 }

Also applies to: 21-21


54-63: Debug实现中提供更多有用信息

当前Debug实现中,只显示了package_json的路径信息。为了调试的完整性,可能需要包含PackageJson的其他关键信息,或明确指出当package_jsonNone时的情况。

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

52-73: 建议为 resolve_id 方法添加文档注释

为了提高代码的可维护性和可读性,建议为 resolve_id 方法添加相应的文档注释,解释其功能、参数和返回值。

应用以下差异以添加文档注释:

+    /// 解析模块标识符。
     fn resolve_id(
         &self,
         source: &str,
         importer: &str,
         _context: &Arc<Context>,
     ) -> Result<Option<ResolverResource>> {
crates/mako/src/plugin.rs (3)

56-63: 建议为新增方法添加文档注释

为了提高代码的可读性和维护性,建议为 resolve_id 方法添加文档注释,说明其作用和使用方式。


255-268: 建议为新增方法添加文档注释

为了提高代码的可读性和维护性,建议为 resolve_id 方法添加文档注释,说明其功能和预期行为。


56-63: 建议为新增的 resolve_id 方法编写单元测试

为了确保新添加的 resolve_id 方法在各种情况下都能正常工作,建议为其编写相应的单元测试,覆盖不同的输入场景和边界条件。

Also applies to: 255-268

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e0fbedf and ae63881.

📒 Files selected for processing (12)
  • crates/binding/src/js_hook.rs (3 hunks)
  • crates/binding/src/js_plugin.rs (3 hunks)
  • crates/mako/src/lib.rs (1 hunks)
  • crates/mako/src/plugin.rs (2 hunks)
  • crates/mako/src/resolve.rs (3 hunks)
  • crates/mako/src/resolve/resolution.rs (1 hunks)
  • crates/mako/src/resolve/resource.rs (1 hunks)
  • e2e/fixtures/plugins/expect.js (1 hunks)
  • e2e/fixtures/plugins/plugins.config.js (1 hunks)
  • e2e/fixtures/plugins/resolve_id_mock.js (1 hunks)
  • e2e/fixtures/plugins/src/index.tsx (1 hunks)
  • packages/mako/binding.d.ts (2 hunks)
✅ Files skipped from review due to trivial changes (2)
  • e2e/fixtures/plugins/resolve_id_mock.js
  • e2e/fixtures/plugins/src/index.tsx
🧰 Additional context used
🔇 Additional comments (12)
crates/mako/src/lib.rs (1)

18-18: resolve 模块设为公共可见

这个更改使 resolve 模块对外部模块可见。这与 PR 目标中提到的新增 resolve_id 插件钩子相符。

请运行以下脚本以验证 resolve 模块的使用:

✅ Verification successful

已验证 resolve 模块的可见性更改

该更改成功将 resolve 模块设为公共可见,并且没有发现任何外部使用情况。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:验证 `resolve` 模块的使用情况

# 测试:搜索 `resolve` 模块的使用。预期:找到新的外部使用情况。
rg --type rust "use (crate|super)::resolve"

Length of output: 1432

crates/mako/src/resolve/resource.rs (1)

3-3: 导入更改看起来没有问题,但需要进一步验证

这个更改将 Resolution 类型的导入从外部 crate 改为本地模块,这看起来是个合理的改动。然而,我们需要确保:

  1. 新的本地 Resolution 类型与之前的实现完全兼容。
  2. 代码库中其他使用 oxc_resolver::Resolution 的地方都已更新。
  3. 如果 Resolution 类型的实现有任何变化,相关文档已经更新。

请运行以下脚本来验证更改的影响:

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

Line range hint 54-66: 总体评价:变更符合预期且集成良好

这些修改很好地实现了 PR 的目标,即添加 resolve_id 插件钩子。新增的 resolveId 方法和 ResolveIdResult 接口为插件系统提供了一种清晰的机制来参与标识符解析过程。这些变更与现有代码库的结构和类型定义保持一致,不会破坏现有功能。

总的来说,这些更改增强了插件系统的灵活性,允许更精细的控制over标识符解析过程。建议在合并这些更改之前,确保更新相关文档,以反映这个新的插件钩子。

crates/mako/src/resolve/resolution.rs (1)

41-51: 验证full_path方法在不同平台下的正确性

full_path方法中,通过将queryfragment直接使用path.push附加到路径上。请确认这种方式在所有平台(尤其是Windows和Unix系统)下都能正确拼接路径,避免出现路径格式错误。

您可以考虑在不同平台上运行测试,或使用路径处理库来确保跨平台兼容性。

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

52-73: 新增的 resolve_id 方法实现正确

该方法正确地调用了 resolve_id 钩子函数,并正确处理了返回值,逻辑清晰,符合预期。

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

61-62: 在 JsHooks 结构体中添加 resolve_id 字段

新增的 resolve_id 字段,以及对应的 TypeScript 类型声明,符合预期,实现合理。


69-69: 在 TsFnHooks 结构体中添加 resolve_id 字段

新增的 resolve_id 字段,类型为 Option<ThreadsafeFunction<(String, String), Option<ResolveIdResult>>>,与 JsHooks 中的定义保持一致,实现合理。


85-87: 在 TsFnHooks::new 方法中正确初始化 resolve_id 字段

resolve_id 字段的初始化方式与其他钩子函数一致,实现合理。


109-112: 新增 ResolveIdResult 结构体

定义了新的结构体 ResolveIdResult,包含一个 id: String 字段,标记为可为空对象,符合预期。

crates/mako/src/resolve.rs (3)

13-13: 确认新模块的引入和导出

在第13行引入了新的模块 mod resolution;,并在第15行通过 pub use resolution::Resolution; 公开了 Resolution。这确保了其他模块可以访问和使用 Resolution 结构体。

Also applies to: 15-15


257-262: 确保 Resolution 结构体的字段正确赋值

在第257-262行,将 resolution 转换为新的 Resolution 结构体。请确保以下字段正确赋值:

  • package_json: 使用 cloned() 方法克隆可能的 package_json,以确保所有权正确。
  • path: 调用 into_path_buf() 将路径转换为 PathBuf 类型。
  • queryfragment: 使用 map 方法并调用 to_string(),以处理可能存在的查询和片段部分。

55-60: 检查 resolve_id 方法的错误处理

在第55-60行,添加了对插件驱动的 resolve_id 方法的调用,并使用 ? 操作符处理可能的错误。如果 resolve_id 返回错误,整个 resolve 函数将提前返回错误。请确认这种错误传播方式符合预期,或者是否需要在此处进行错误捕获和处理以增强鲁棒性。

为验证 resolve_id 方法的错误处理方式,请运行以下脚本查看其定义:

✅ Verification successful

继续检查 resolve_id 方法的错误处理

为了进一步确认 resolve_id 方法的错误传播方式,请运行以下脚本以获取其实现细节:


确认 resolve_id 方法的错误处理符合预期

所有 resolve_id 方法均通过 ? 操作符传播错误,符合预期的错误处理方式。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:查找 `resolve_id` 方法的定义以确认错误处理方式

# 测试:搜索插件驱动中 `resolve_id` 方法的定义。期望:找到方法定义的位置。
rg --type rust 'fn resolve_id\(' -A 5

Length of output: 1025


Script:

# 
#!/bin/bash
# 描述:提取 `resolve_id` 方法的实现以检查错误处理逻辑

rg --type rust 'fn resolve_id\(' -A 20

Length of output: 3535

Comment on lines +7 to +13
#[derive(Clone)]
pub struct Resolution {
pub path: PathBuf,
pub query: Option<String>,
pub fragment: Option<String>,
pub package_json: Option<Arc<PackageJson>>,
}
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

建议将结构体字段设为私有以增强封装性

当前,Resolution结构体的所有字段都声明为pub,这可能会暴露内部实现细节。建议将这些字段设为私有,并提供公有的访问方法(getter),以保持封装性和灵活性。

Comment on lines +36 to +39
/// Returns serialized package_json
pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
self.package_json.as_ref()
}
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

更新方法注释以反映实际返回值

package_json方法的注释为“返回序列化的package_json”,但实际上返回的是PackageJson的引用。为了避免误导,建议修改注释以准确描述方法的返回值。

修改建议:

- /// 返回序列化的package_json
+ /// 返回与路径关联的`PackageJson`引用(如果存在)
 pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
     self.package_json.as_ref()
 }
📝 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
/// Returns serialized package_json
pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
self.package_json.as_ref()
}
/// 返回与路径关联的`PackageJson`引用(如果存在)
pub fn package_json(&self) -> Option<&Arc<PackageJson>> {
self.package_json.as_ref()
}

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 59.25926% with 22 lines in your changes missing coverage. Please review.

Project coverage is 61.97%. Comparing base (e0fbedf) to head (fd5afd1).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
crates/mako/src/resolve/resolution.rs 41.17% 20 Missing ⚠️
crates/mako/src/plugin.rs 90.90% 1 Missing ⚠️
crates/mako/src/resolve.rs 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1625      +/-   ##
==========================================
- Coverage   61.98%   61.97%   -0.02%     
==========================================
  Files         154      155       +1     
  Lines       15414    15467      +53     
==========================================
+ Hits         9554     9585      +31     
- Misses       5860     5882      +22     

☔ 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: 0

🧹 Outside diff range and nitpick comments (3)
docs/config.md (3)

547-547: 新增的 resolveId 钩子函数看起来不错

resolveId 钩子函数的添加与 PR 的目标一致,为插件系统提供了解析模块标识符的能力。这是一个有用的补充。

建议:

  1. 考虑为 resolveId 函数添加更详细的说明,解释其用途和使用场景。
  2. 可以提供一个简单的示例,展示如何在插件中使用这个新的钩子函数。

Line range hint 1037-1062: 关于别名配置的新增说明非常有帮助

这些新增的说明很好地解释了如何正确使用别名功能,有助于防止用户犯常见错误。

建议:

  1. 考虑将这两个注意事项合并成一个列表,以提高可读性。
  2. 可以添加一个正确配置的完整示例,展示如何同时避免这两个问题。

Line range hint 1-1236: 配置文档整体结构清晰,内容全面

这份配置文档提供了 Mako 的全面配置指南,结构清晰,解释详细,对用户非常有帮助。新增的内容与现有文档风格一致,很好地融入了整体结构。

建议:

  1. 考虑在文档开头添加一个目录或快速导航,方便用户快速找到所需的配置项。
  2. 可以考虑为一些较复杂的配置项添加更多实际应用场景的示例。
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between ae63881 and b133fa3.

📒 Files selected for processing (2)
  • docs/config.md (1 hunks)
  • docs/config.zh-CN.md (1 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
  • docs/config.zh-CN.md
🧰 Additional context used

path: PathBuf::from(x.id),
query: None,
fragment: None,
package_json: None,
Copy link
Member

Choose a reason for hiding this comment

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

需要支持下 package json 的内容的传递,以保证需要使用报名的场景 or package.json 中 side effects

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: 0

🧹 Outside diff range and nitpick comments (7)
e2e/fixtures/plugins/plugins.config.js (1)

22-34: 新的 resolveId 方法实现看起来不错,但请考虑在生产环境中移除日志记录。

resolveId 方法的实现符合预期,正确处理了 'resolve_id' 和 'resolve_id_external' 两种情况。使用 require('path').join 构建文件路径是个好做法。然而,在第 25 行的 console.log 可能会在生产环境中造成不必要的日志输出。

建议在生产环境中移除或禁用 console.log 语句。您可以考虑使用条件语句来控制日志输出,例如:

if (process.env.NODE_ENV !== 'production') {
  console.log('resolveId', source, importer);
}

这样可以确保在开发过程中保留有用的调试信息,同时避免在生产环境中产生不必要的日志。

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

61-62: 新增的 resolve_id 钩子函数符合预期

这个新增的 resolve_id 字段很好地实现了 PR 的目标。它的类型定义和其他钩子保持一致,使用 Option<JsFunction> 允许该钩子是可选的。TypeScript 类型注解也正确地描述了预期的函数签名。

建议为这个新字段添加一个简短的文档注释,解释其用途和预期行为。例如:

/// 解析给定源和导入者的标识符。
#[napi(ts_type = "(source: string, importer: string) => Promise<{ id: string }>;")]
pub resolve_id: Option<JsFunction>,

69-69: TsFnHooks 中的 resolve_id 字段实现正确

新增的 resolve_id 字段在 TsFnHooks 结构体中的实现是正确的。它使用了 ThreadsafeFunction,与结构体中的其他字段保持一致,确保了并发访问的安全性。输入和输出类型的定义也符合预期的功能。

建议为这个新字段添加一个简短的文档注释,解释其用途和与 JsHooks 中对应字段的关系。例如:

/// 对应于 JsHooks 中的 resolve_id 字段的线程安全版本。
pub resolve_id: Option<ThreadsafeFunction<(String, String), Option<ResolveIdResult>>>,

109-113: ResolveIdResult 结构体定义合理

新增的 ResolveIdResult 结构体很好地封装了 resolve_id 操作的结果。id 字段为必需项,而 external 字段为可选项,这种设计提供了足够的灵活性。使用 #[napi(object, use_nullable = true)] 注解也确保了与 JavaScript 表示的兼容性。

建议添加一些文档注释来解释这个结构体的用途和字段的含义。例如:

/// 表示 resolve_id 操作的结果。
#[napi(object, use_nullable = true)]
pub struct ResolveIdResult {
    /// 解析后的标识符。
    pub id: String,
    /// 可选字段,指示解析的标识符是否为外部资源。
    pub external: Option<bool>,
}

此外,考虑使用 #[must_use] 属性来提醒调用者不要忽略这个结构体的实例。

docs/config.md (3)

547-547: 新增的 resolveId 钩子函数看起来不错

新增的 resolveId 钩子函数很好地补充了插件系统的功能,允许开发者自定义模块标识符的解析过程。这与 PR 的目标相符,即引入新的 resolve_id 插件钩子。

建议为 resolveId 钩子函数添加一个简短的示例,以帮助用户更好地理解其用法。例如:

resolveId: async (id: string, importer: string) => {
  if (id === 'virtual-module') {
    return { id: '/absolute/path/to/module.js', external: false };
  }
  return null; // 返回 null 表示使用默认解析
}

Line range hint 1022-1046: 解析配置说明的补充很有帮助

新增的两条注意事项为用户提供了重要的配置指导,有助于防止常见的别名配置错误。这些补充说明非常有价值。

为了提高可读性,建议将这两条注意事项格式化为一个列表。例如:

注意:
1. 如果要为目录设置别名,请不要添加 `/*` 后缀,我们目前不支持这种方式。
2. 如果要为本地路径设置别名,请确保添加 `./` 前缀。否则,它将被视为依赖模块。

例如:

```diff
{
  resolve: {
    alias: [
-      ["@/src/*", "./src/*"],
+      [ "@/src", "./src"],
-      ["@/src", "src"],
+      ["@/src", "./src"],
    ],
  },
}

这样的格式更容易阅读和理解。

---

Line range hint `1-1137`: **文档的整体更新非常全面和有价值**

这次更新大大提高了 Mako 配置文档的质量和完整性。新增的内容和现有结构保持一致,同时提供了更多的细节和澄清。这对用户理解和正确配置 Mako 非常有帮助。



为了保持文档风格的一致性,建议检查所有配置项的描述格式。例如,确保每个配置项都有类型、默认值和简短描述。可以考虑使用表格或统一的列表格式来呈现这些信息,这样可以让文档更易于浏览和查找特定信息。

此外,可以考虑在文档开头添加一个目录,列出所有主要配置项,并提供快速跳转链接。这将帮助用户更快地找到他们需要的特定配置信息。

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**

<details>
<summary>📥 Commits</summary>

Files that changed from the base of the PR and between b133fa32f8314b9df3365c37a319c0706c73532c and fd5afd137ee28c4a3142666e5423dea3d5b488c3.

</details>

<details>
<summary>📒 Files selected for processing (8)</summary>

* crates/binding/src/js_hook.rs (3 hunks)
* crates/binding/src/js_plugin.rs (3 hunks)
* docs/config.md (1 hunks)
* docs/config.zh-CN.md (1 hunks)
* e2e/fixtures/plugins/expect.js (1 hunks)
* e2e/fixtures/plugins/plugins.config.js (1 hunks)
* e2e/fixtures/plugins/src/index.tsx (1 hunks)
* packages/mako/binding.d.ts (2 hunks)

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (4)</summary>

* crates/binding/src/js_plugin.rs
* e2e/fixtures/plugins/expect.js
* e2e/fixtures/plugins/src/index.tsx
* packages/mako/binding.d.ts

</details>

<details>
<summary>🧰 Additional context used</summary>





</details>

<details>
<summary>🔇 Additional comments (7)</summary><blockquote>

<details>
<summary>docs/config.zh-CN.md (7)</summary><blockquote>

`547-550`: **新增配置项:emitDecoratorMetadata**

这是一个有用的新增配置项,允许用户控制是否输出装饰器元数据。

建议添加一个简短的说明,解释这个选项的用途,例如:
"此选项用于启用TypeScript装饰器元数据的输出,对于使用反射或依赖注入框架的项目特别有用。"

---

Line range hint `552-555`: **新增配置项:emotion**

这是一个很好的新增配置项,为使用Emotion的项目提供了支持。

建议添加一个简短的说明,解释Emotion是什么,例如:
"Emotion是一个流行的CSS-in-JS库,启用此选项可以在项目中使用Emotion的样式方案。"

---

Line range hint `557-576`: **新增实验性配置项:experimental.detectLoop**

这是一个非常有用的新增实验性功能,可以帮助开发者检测项目中的依赖循环。

建议:
1. 在配置项描述的开头更明显地标注这是一个实验性功能,例如:
   "【实验性功能】用于检测依赖循环的配置。"
2. 添加一个简短的说明,解释依赖循环检测的重要性,例如:
   "依赖循环可能导致意外的行为和性能问题,及时发现和解决它们对于维护健康的代码库很重要。"
3. 考虑添加一个使用示例,说明如何解释生成的graphviz文件。

---

Line range hint `578-590`: **新增实验性配置项:experimental.requireContext**

这个新增的实验性配置项提供了对`require.context`功能的控制,这是一个很好的灵活性增强。

建议:
1. 在配置项描述的开头更明显地标注这是一个实验性功能。
2. 提供更多关于`require.context`功能的背景信息,例如:
   "require.context是一个用于模块动态导入的webpack特性,它允许您创建一个上下文,从而实现按需导入文件。"
3. 解释为什么默认启用这个功能,以及禁用它可能带来的影响。
4. 考虑添加一个使用示例,展示如何在项目中利用这个功能。

---

Line range hint `592-605`: **新增实验性配置项:experimental.webpackSyntaxValidate**

这个新增的实验性配置项为使用webpack语法的包提供了一个白名单机制,这是一个有趣的功能。

建议:
1. 在配置项描述的开头更明显地标注这是一个实验性功能。
2. 提供更多关于webpack语法的背景信息,例如:
   "webpack语法包括特定的导入语句、魔法注释等,这些在非webpack环境中可能不被支持。"
3. 解释使用这个选项的潜在影响和风险,例如:
   "允许使用webpack语法可能会影响项目的可移植性和与其他构建工具的兼容性。"
4. 考虑添加一个使用示例,展示如何正确配置这个选项。

---

`547-547`: **更新的配置项:plugins.resolveId**

在plugins配置中新增的`resolveId`钩子函数是一个强大的扩展点,可以让插件参与模块解析过程。

建议:
1. 提供更多关于`resolveId`钩子的使用场景和目的,例如:
   "此钩子允许插件自定义模块解析逻辑,对于处理别名、虚拟模块或特殊的导入路径特别有用。"
2. 解释返回值中`external`属性的作用,例如:
   "当`external`为true时,表示该模块应被视为外部依赖,不应被打包。"
3. 考虑添加一个简单的使用示例,展示如何在插件中实现这个钩子函数。

---

Line range hint `1009-1013`: **更新的配置项:publicPath**

关于运行时设置`publicPath`的说明是一个很好的补充,提高了配置的灵活性。

建议:
1. 提供一个简短的代码示例,展示如何在运行时使用`__mako_public_path__`,例如:
   ```javascript
   // 在应用的入口文件中
   __mako_public_path__ = process.env.PUBLIC_PATH;
  1. 解释为什么同时支持__mako_public_path____webpack_public_path__,例如:
    "为了保持与webpack项目的兼容性,我们同时支持这两种设置方式。"
  2. 提醒用户在使用运行时设置时可能需要注意的事项,如确保在加载任何资源之前设置publicPath

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.

2 participants