-
Notifications
You must be signed in to change notification settings - Fork 85
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 writeBundle plugin hook #1650
Conversation
Walkthrough此次更改在多个文件中引入了新的 Changes
Possibly related PRs
Suggested reviewers
Poem
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 (
|
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: 1
🧹 Outside diff range and nitpick comments (8)
e2e/fixtures/plugins/plugins.config.js (1)
11-13
: 新增的writeBundle
方法看起来不错,但可以考虑进一步完善。这个新方法正确地定义为异步方法,符合打包操作的异步特性。目前的实现仅包含一个控制台日志,这可能是出于调试或演示目的。
考虑以下建议来增强此方法:
添加参数以接收打包信息,例如:
async writeBundle(bundle) { console.log("writeBundle", bundle); }添加错误处理,以便在实际使用时更加健壮:
async writeBundle(bundle) { try { console.log("writeBundle", bundle); // 执行实际的打包后操作 } catch (error) { console.error("writeBundle error:", error); } }这些改进将使该方法在未来的实际应用中更加实用和可靠。
crates/binding/src/js_plugin.rs (1)
124-129
: 新方法实现正确,建议小改进
write_bundle
方法的实现看起来正确且符合其他方法的模式。它正确处理了钩子可能不存在的情况,并使用了一致的错误处理方式。建议考虑以下小改进:
-fn write_bundle(&self, _context: &Arc<Context>) -> Result<()> { +fn write_bundle(&self, context: &Arc<Context>) -> Result<()> { if let Some(hook) = &self.hooks.write_bundle { - hook.call(())? + hook.call(context)? } Ok(()) }这样可以将
context
传递给钩子函数,以便在需要时使用。如果确定不需要context
,可以保持现状。crates/binding/src/js_hook.rs (3)
60-61
: 新增的 write_bundle 字段看起来不错新增的
write_bundle
字段与 PR 的目标一致,类型和可选性也很合适。为了提高代码的可维护性,建议添加一个简短的注释来解释这个钩子的用途。建议添加如下注释:
/// 在写入 bundle 文件之前调用的钩子函数 pub write_bundle: Option<JsFunction>,
83-83
: TsFnHooks 结构体中的 write_bundle 字段添加正确
TsFnHooks
结构体中新增的write_bundle
字段与JsHooks
结构体中的变更相对应,使用ThreadsafeFunction
也与其他字段保持一致。为了提高代码的可读性,建议添加一个简短的注释。建议添加如下注释:
/// 线程安全的 write_bundle 钩子函数 pub write_bundle: Option<ThreadsafeFunction<(), ()>>,
103-105
: TsFnHooks::new 方法中 write_bundle 的初始化看起来正确
write_bundle
字段的初始化与其他字段保持一致,使用了相同的模式从JsHooks
映射到TsFnHooks
。不过,建议考虑改进错误处理方式。考虑使用
expect
替代unwrap
,以提供更有意义的错误信息:write_bundle: hooks.write_bundle.as_ref().map(|hook| unsafe { ThreadsafeFunction::from_napi_value(env.raw(), hook.raw()) .expect("Failed to create ThreadsafeFunction for write_bundle") }),这样可以在出错时提供更有用的调试信息。
packages/mako/binding.d.ts (1)
54-54
: 新增的 writeBundle 方法看起来不错这个新增的可选方法
writeBundle
扩展了JsHooks
接口的功能,并且与接口中其他返回Promise<void>
的异步方法保持一致。它的名称暗示了与写入打包文件相关的功能,这很可能是构建过程的一部分。建议为这个新方法添加 JSDoc 注释,以解释其用途和在构建过程中的调用时机。例如:
/** * 在打包过程结束后调用,用于执行额外的打包文件写入操作。 * @returns 一个 Promise,在写入操作完成时解决。 */ writeBundle?: () => Promise<void>;crates/mako/src/plugin.rs (1)
331-336
:PluginDriver
中的write_bundle
实现看起来不错,但可以考虑优化。这个实现正确地遍历了所有插件并调用它们的
write_bundle
方法。错误处理也很恰当,如果任何插件的write_bundle
调用失败,它会停止并返回错误。建议优化:考虑使用
Iterator::try_for_each
来简化代码。例如:pub fn write_bundle(&self, context: &Arc<Context>) -> Result<()> { self.plugins.iter().try_for_each(|plugin| plugin.write_bundle(context)) }这样可以使代码更简洁,同时保持相同的功能。
docs/config.zh-CN.md (1)
574-574
: 新增的writeBundle
钩子函数文档看起来不错新增的
writeBundle
钩子函数是一个很好的补充,它为插件开发者提供了在bundle写入后执行操作的能力。这与PR的目标相符,增强了插件系统的功能。为了使文档更加清晰,建议添加一个简短的描述,说明这个钩子函数的用途和触发时机。例如:
- writeBundle?: () => void; + writeBundle?: () => void; // 在bundle写入磁盘后调用,可用于执行额外的文件操作或通知
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- crates/binding/src/js_hook.rs (3 hunks)
- crates/binding/src/js_plugin.rs (1 hunks)
- crates/mako/src/compiler.rs (1 hunks)
- crates/mako/src/dev.rs (1 hunks)
- crates/mako/src/plugin.rs (2 hunks)
- docs/config.md (1 hunks)
- docs/config.zh-CN.md (1 hunks)
- e2e/fixtures/plugins/plugins.config.js (1 hunks)
- packages/mako/binding.d.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (7)
crates/binding/src/js_hook.rs (1)
Line range hint
1-165
: 总体评价:write_bundle 钩子的实现看起来不错新增的
write_bundle
钩子在JsHooks
和TsFnHooks
结构体中都得到了正确的实现,并在TsFnHooks::new
方法中进行了适当的初始化。这些更改与文件中现有的模式保持一致,很好地实现了 PR 的目标。有几点小的改进建议:
- 为新增的字段添加简短的注释,以提高代码可读性。
- 在
TsFnHooks::new
方法中,考虑使用expect
替代unwrap
来提供更有意义的错误信息。总的来说,这些更改质量很高,与现有代码风格保持一致。在应用了建议的小改进后,这个实现将会更加完善。
crates/mako/src/plugin.rs (1)
150-152
: 新增的write_bundle
方法看起来不错。这个方法为插件提供了一个写入打包结果的钩子。默认实现返回
Ok(())
,允许插件在不需要此功能时选择退出。使用Arc<Context>
参数表明上下文可以在线程间安全共享,这对于并发操作是有利的。docs/config.md (4)
576-576
: 新增writeBundle
钩子函数的文档
writeBundle
钩子函数的文档已添加到插件配置中。这是一个重要的新功能,可以在打包完成后执行自定义操作。建议考虑添加一个简单的示例,以帮助用户更好地理解如何使用这个新的钩子函数。例如:
plugins: [ { name: 'my-plugin', writeBundle: () => { console.log('Bundle has been written to disk'); // 在这里执行打包后的自定义操作 } } ]
576-576
:publicPath
配置说明的改进
publicPath
配置的文档已得到显著改进,新增了关于 "runtime" 特殊值的说明,以及如何在运行时设置publicPath
的指导。这些信息对需要动态设置publicPath
的用户来说非常有价值。为了进一步提高文档的可读性,建议考虑添加一个简单的代码示例,展示如何在实际场景中使用
__mako_public_path__
。例如:// 在应用的入口文件中 if (process.env.NODE_ENV === 'production') { __mako_public_path__ = 'https://cdn.example.com/assets/'; } else { __mako_public_path__ = '/dev-assets/'; }
576-576
:resolve
配置说明的重要补充
resolve
配置的文档新增了两个重要的注意事项:
- 不要为目录别名添加
/*
后缀- 本地路径别名必须添加
./
前缀这些补充信息对于防止用户在配置别名时犯常见错误非常有价值。
为了使文档更加完整,建议考虑添加一个"正确示例"和"错误示例"的对比,以直观地展示这些注意事项。例如:
// 正确示例 alias: [ ["@", "./src"], ["utils", "./src/utils"] ] // 错误示例 alias: [ ["@/*", "./src/*"], // 不要使用 /* 后缀 ["utils", "src/utils"] // 缺少 ./ 前缀 ]
576-576
: 文档更新总结本次更新对
docs/config.md
文件进行了多处有价值的改进:
- 新增了
writeBundle
插件钩子的说明- 扩展了
publicPath
配置的解释,包括运行时设置的方法- 在
resolve
配置中添加了重要的别名设置注意事项这些更新显著提高了文档的完整性和实用性,将有助于用户更好地理解和使用 Mako 的配置选项。
建议考虑在文档的顶部添加一个"最近更新"或"更新日志"部分,以便用户快速了解文档的最新变化。
crates/mako/src/compiler.rs (1)
463-463
: 在编译流程中正确添加了write_bundle
调用在
compile
方法中适当地调用了write_bundle
,确保插件的打包内容被正确写入。
compiler | ||
.context | ||
.plugin_driver | ||
.write_bundle(&compiler.context) | ||
.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.
新增的 write_bundle 调用需要错误处理
新增的 write_bundle
调用是一个很好的改进,它与PR的整体目标一致。然而,当前实现缺少错误处理机制。
建议添加错误处理,例如:
compiler
.context
.plugin_driver
.write_bundle(&compiler.context)
.map_err(|e| {
debug!("写入 bundle 时发生错误: {:?}", e);
e
})?;
这样可以确保在写入过程中发生的任何错误都能被适当地处理和记录。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1650 +/- ##
==========================================
+ Coverage 55.56% 55.57% +0.01%
==========================================
Files 172 172
Lines 17418 17429 +11
==========================================
+ Hits 9678 9687 +9
- Misses 7740 7742 +2 ☔ View full report in Codecov by Sentry. |
.
Summary by CodeRabbit
新功能
JsHooks
和TsFnHooks
结构中添加了可选的write_bundle
字段。JsPlugin
结构中新增write_bundle
方法,支持插件处理捆绑写入。DevServer
的rebuild
方法更新,重建后写入捆绑。文档