feat: move ensure runtime to entry #1660
Conversation
Walkthrough此次更改在编译器的插件系统中引入了新的插件 Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
crates/mako/src/plugins/ensure_2.rs (1)
11-13: 建议为插件名称添加文档注释插件名称包含 "dev_" 前缀表明这是一个开发特性。建议添加文档注释说明这一点，以及何时会移除此前缀。
建议添加如下文档：
/// Ensure2 插件的开发版本。 /// 注意：当前带有 dev_ 前缀，表示这是一个实验性特性。 impl Plugin for Ensure2 {crates/mako/src/visitors/dynamic_import.rs (2)
109-111: 移除未使用的变量_chunk_ids以优化代码变量
_chunk_ids被声明但未在后续代码中使用。建议删除该变量和相关的计算逻辑，以简化代码并避免不必要的计算。
179-185: 调整方法顺序以提高代码可读性建议将
make_load_promise_2方法和make_load_promise方法的顺序进行调整，使相关的方法按逻辑顺序排列，增强代码的可读性和维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- crates/mako/src/compiler.rs (1 hunks)
- crates/mako/src/plugins.rs (1 hunks)
- crates/mako/src/plugins/ensure_2.rs (1 hunks)
- crates/mako/src/visitors/dynamic_import.rs (4 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/mako/src/plugins.rs
🔇 Additional comments (7)
crates/mako/src/plugins/ensure_2.rs (3)
1-8: 代码结构清晰，导入合理！导入的模块和结构体声明符合 Rust 最佳实践。
1-64: 总体实现符合需求，建议进行性能测试代码实现了将 ensure runtime 移至 entry 的目标，简化了动态导入的处理。但由于这会增加 entry chunk 的大小，建议：
- 添加性能基准测试
- 监控 entry chunk 大小的变化
- 考虑添加配置开关以便灵活控制该特性
运行以下命令分析对 entry chunk 的影响：
#!/bin/bash # 搜索相关的性能测试代码 rg "bench|test.*ensure2|chunk.*size" -t rust
43-60: 需要处理 TODO 并改进代码生成
- 关于 TODO 的压缩优化：建议实现一个共享依赖的压缩算法，可以显著减少地图大小。
- JavaScript 代码生成可以改进：
- 考虑使用更安全的 JSON 序列化方式
- 添加错误处理的兜底逻辑
运行以下命令检查地图大小：
是否需要我帮助实现地图压缩算法？
crates/mako/src/compiler.rs (2)
Line range hint
249-255: 验证运行时插件的依赖关系当前的插件顺序为：
- HMRRuntimePlugin
- Ensure2
- WasmRuntimePlugin
- AsyncRuntimePlugin
这个顺序需要确保：
- Ensure2 的转换不会影响其他运行时插件的功能
- 不会与现有的 HMR 功能产生冲突
#!/bin/bash # 检查运行时插件之间的依赖关系 ast-grep --pattern 'impl Plugin for $plugin { $$$ fn $method(&self, $_: &Context) -> Result<()> { $$$ ensure_2 $$$ } $$$ }'
251-251: 确认 Ensure2 插件的添加位置插件的位置安排合理，在其他运行时插件之前。这符合动态导入处理的要求，确保模块转换在分块分组之前完成。但需要注意以下几点：
- 由于所有映射将被移至入口分块，这可能导致入口分块大小增加
- 建议先在开发环境中启用此功能，评估其影响
✅ Verification successful
Ensure2 插件的位置和实现验证完成
经过验证，Ensure2 插件的实现和位置是合适的：
- 插件实现了异步块的依赖管理，通过
runtime_plugins注入运行时代码- 在编译器插件链中的位置合理，位于
WasmRuntimePlugin和AsyncRuntimePlugin之前- 不会对其他插件造成负面影响，因为它只处理异步块的依赖关系
该插件通过
requireModule.ensure2优化了动态导入的处理方式，将相关依赖映射集中到入口分块是合理的实现策略。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否有其他插件依赖 Ensure2 的输出 rg -l "ensure_2::" --type rustLength of output: 145
Script:
#!/bin/bash # 检查 ensure_2 插件的实现和依赖关系 rg -g "*.rs" "ensure_2" -A 5 -B 5 # 查找其他可能使用 ensure_2 的模块 ast-grep --pattern 'use $$$ensure_2$$$' # 检查插件的执行顺序相关代码 rg -g "*.rs" "Plugin.*order" -A 5 -B 5Length of output: 1661
Script:
#!/bin/bash # 检查 ensure_2 插件的具体实现 cat crates/mako/src/plugins/ensure_2.rs # 检查插件的依赖关系 ast-grep --pattern 'struct Ensure2 { $$$ }' # 查找插件的调用点 ast-grep --pattern 'fn apply(&self, $_: &Context) -> Result<()> { $$$ }'Length of output: 2093
crates/mako/src/visitors/dynamic_import.rs (2)
77-77: 确保动态导入的处理完整性在第 77 行附近，请检查对动态导入表达式的处理是否涵盖了所有可能的情况，包括潜在的错误和异常，以确保代码的健壮性。
151-152: 🛠️ Refactor suggestion确认
make_load_promise方法的必要性在第 151 行，原先调用的
self.make_load_promise(&chunk_ids)被注释掉，替换为self.make_load_promise_2(resolved_source)。如果make_load_promise方法不再使用，建议删除该方法以减少冗余代码。
| *expr = { | ||
| // let load_promise = self.make_load_promise(&chunk_ids); | ||
| let load_promise = self.make_load_promise_2(resolved_source); | ||
|
|
||
| let lazy_require_call = | ||
| member_expr!(DUMMY_CTXT, DUMMY_SP, __mako_require__.bind).as_call( | ||
| DUMMY_SP, | ||
| vec![ | ||
| quote_ident!("__mako_require__").as_arg(), | ||
| quote_str!(require_module.as_str()).as_arg(), | ||
| quote_str!(resolved_source.clone()).as_arg(), | ||
| ], | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
优化新表达式的构建过程
在构建新的表达式时，可以考虑简化 lazy_require_call 和 dr_call 的生成逻辑，避免重复代码，从而提高代码效率。
|
周会记录：需要考虑 update 阶段的 mapping 关系如何更新到运行时 |
|
update 阶段 简单的方案：
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1660 +/- ##
==========================================
- Coverage 55.63% 55.42% -0.22%
==========================================
Files 173 174 +1
Lines 17530 17621 +91
==========================================
+ Hits 9753 9766 +13
- Misses 7777 7855 +78 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (12)
crates/mako/src/config/experimental.rs (1)
13-13: 建议为新的实验性功能添加文档注释考虑到这是一个实验性功能，建议添加文档注释来说明：
- 功能的用途和预期行为
- 使用场景和限制
- 对构建产物大小的潜在影响
建议添加如下文档：
+ /// 启用集中式 ensure 机制，将动态导入的映射关系集中到入口文件 + /// + /// # 注意 + /// - 此功能会增加入口文件的大小 + /// - 建议仅在开发环境中启用以评估影响 pub central_ensure: bool,crates/mako/src/config/mako.config.default.json (1)
74-75: 建议：添加配置文档建议为这个新的实验性功能添加相关文档，说明：
- 功能的目的和影响
- 对构建产物大小的影响
- 何时应该启用/禁用此功能
需要我帮助生成文档模板吗？
crates/mako/src/visitors/dynamic_import.rs (3)
50-59: 建议增强错误处理机制当前代码使用
unwrap()直接获取@swc/helpers/_/_interop_require_wildcard的值。建议添加适当的错误处理，以防止在缺少该依赖时的潜在崩溃。- let interop_replace = self - .dep_to_replace - .resolved - .get("@swc/helpers/_/_interop_require_wildcard") - .unwrap(); + let interop_replace = self + .dep_to_replace + .resolved + .get("@swc/helpers/_/_interop_require_wildcard") + .ok_or_else(|| format_err!("Missing required helper: @swc/helpers/_/_interop_require_wildcard"))?;
158-188: 建议优化 chunk 图的处理逻辑
inline_ensure方法中的 chunk 图处理逻辑较为复杂：
- 建议添加详细的文档说明，特别是关于 chunk 依赖关系的处理逻辑
- 考虑将 chunk 图的处理逻辑抽取为独立的私有方法，提高代码可维护性
- 可以考虑使用 Iterator 链式操作优化代码结构
+ /// Retrieves chunk IDs for a given module, including its sync dependencies. + /// Returns an empty vector if the chunk has been optimized to entry chunk. + fn get_chunk_ids(&self, module_id: &ModuleId, chunk_graph: &ChunkGraph) -> Vec<String> { + let chunk = chunk_graph.get_chunk_for_module(module_id); + match chunk { + Some(chunk) => [ + chunk_graph.sync_dependencies_chunk(&chunk.id), + vec![chunk.id.clone()], + ] + .concat() + .into_iter() + .filter(|chunk_id| { + chunk_graph + .chunk(chunk_id) + .is_some_and(|c| !c.modules.is_empty()) + }) + .map(|chunk_id| chunk_id.id) + .collect(), + None => vec![], + } + }🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 179-179: crates/mako/src/visitors/dynamic_import.rs#L179
Added line #L179 was not covered by tests
[warning] 185-185: crates/mako/src/visitors/dynamic_import.rs#L185
Added line #L185 was not covered by tests
249-256: 建议扩展测试用例当前测试数据结构已更新为使用
ResolvedReplaceInfo，但测试覆盖度仍有提升空间：
- 需要添加
central_ensure相关的测试用例- 建议添加边界情况的测试，如：
- 模块解析失败的情况
- chunk 优化到入口 chunk 的情况
- watch 模式下的行为测试
需要我帮助生成这些测试用例吗？
crates/mako/src/plugins/bundless_compiler.rs (1)
93-94: 类型更新合理，建议补充文档注释将 HashMap 的值类型更新为
ResolvedReplaceInfo是个很好的改进。建议为这个关键的数据结构添加文档注释，说明其用途和字段含义。建议添加如下文档注释：
/// 存储模块依赖的解析结果 /// - key: 原始依赖源 /// - value: 解析后的依赖信息，包含目标路径和解析后的模块IDcrates/mako/src/generate/transform.rs (1)
156-168: 辅助函数的重构看起来不错，建议小改进函数重构后使用
ResolvedReplaceInfo使代码结构更加清晰。不过建议将辅助函数的创建逻辑抽取到一个单独的方法中，以提高代码的可维护性。建议重构如下：
fn insert_swc_helper_replace( map: &mut HashMap<String, ResolvedReplaceInfo>, context: &Arc<Context>, ) { - SWC_HELPERS.into_iter().for_each(|h| { - let m_id: ModuleId = h.to_string().into(); - map.insert( - m_id.id.clone(), - ResolvedReplaceInfo { - to_replace_source: m_id.generate(context), - resolved_module_id: m_id, - }, - ); - }); + for helper in SWC_HELPERS { + map.insert( + helper.to_string(), + create_helper_replace_info(helper, context), + ); + } } + +fn create_helper_replace_info(helper: &str, context: &Arc<Context>) -> ResolvedReplaceInfo { + let module_id: ModuleId = helper.to_string().into(); + ResolvedReplaceInfo { + to_replace_source: module_id.generate(context), + resolved_module_id: module_id, + } +}crates/mako/src/compiler.rs (1)
301-304: 建议添加测试用例当前改动缺少测试覆盖，建议添加以下测试场景：
- watch 模式下启用 central_ensure
- watch 模式下禁用 central_ensure
- 非 watch 模式的行为验证
需要我帮您生成测试代码或创建 GitHub issue 来跟踪这个任务吗？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 302-302: crates/mako/src/compiler.rs#L302
Added line #L302 was not covered by testscrates/mako/src/plugins/central_ensure.rs (3)
52-52: 请处理 TODO 注释，压缩映射以减少重复的 chunk IDs代码中存在
TODO注释，建议实现该功能以提高代码效率和性能。您是否需要协助来实现此功能，或者希望我为此创建一个新的 GitHub issue？
20-20: 改进错误消息格式以提升可读性错误消息应更具可读性，遵循标准命名约定，使用小写字母，并避免使用下划线。
请应用以下修改：
- .map_err(|e| anyhow!("Read_Module_Graph_error:\n{:?}", e))?; + .map_err(|e| anyhow!("Error reading module graph:\n{:?}", e))?;- .map_err(|e| anyhow!("Read_Chunk_Graph_error:\n{:?}", e))?; + .map_err(|e| anyhow!("Error reading chunk graph:\n{:?}", e))?;Also applies to: 24-24
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-21: crates/mako/src/plugins/central_ensure.rs#L20-L21
Added lines #L20 - L21 were not covered by tests
13-75: 考虑为新增代码添加单元测试静态分析工具指出，该文件中的多处新增代码行未被测试覆盖。建议编写相应的单元测试，以确保代码的正确性和稳定性。
您是否需要协助编写这些单元测试？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 13-13: crates/mako/src/plugins/central_ensure.rs#L13
Added line #L13 was not covered by tests
[warning] 15-17: crates/mako/src/plugins/central_ensure.rs#L15-L17
Added lines #L15 - L17 were not covered by tests
[warning] 20-21: crates/mako/src/plugins/central_ensure.rs#L20-L21
Added lines #L20 - L21 were not covered by tests
[warning] 24-24: crates/mako/src/plugins/central_ensure.rs#L24
Added line #L24 was not covered by tests
[warning] 26-26: crates/mako/src/plugins/central_ensure.rs#L26
Added line #L26 was not covered by tests
[warning] 28-30: crates/mako/src/plugins/central_ensure.rs#L28-L30
Added lines #L28 - L30 were not covered by tests
[warning] 32-32: crates/mako/src/plugins/central_ensure.rs#L32
Added line #L32 was not covered by tests
[warning] 34-35: crates/mako/src/plugins/central_ensure.rs#L34-L35
Added lines #L34 - L35 were not covered by tests
[warning] 37-39: crates/mako/src/plugins/central_ensure.rs#L37-L39
Added lines #L37 - L39 were not covered by tests
[warning] 41-42: crates/mako/src/plugins/central_ensure.rs#L41-L42
Added lines #L41 - L42 were not covered by tests
[warning] 44-44: crates/mako/src/plugins/central_ensure.rs#L44
Added line #L44 was not covered by tests
[warning] 46-48: crates/mako/src/plugins/central_ensure.rs#L46-L48
Added lines #L46 - L48 were not covered by tests
[warning] 50-50: crates/mako/src/plugins/central_ensure.rs#L50
Added line #L50 was not covered by tests
[warning] 53-53: crates/mako/src/plugins/central_ensure.rs#L53
Added line #L53 was not covered by tests
[warning] 55-55: crates/mako/src/plugins/central_ensure.rs#L55
Added line #L55 was not covered by tests
[warning] 74-75: crates/mako/src/plugins/central_ensure.rs#L74-L75
Added lines #L74 - L75 were not covered by tests🪛 GitHub Check: Spell Check
[warning] 59-59:
"Maping" should be "Mapping".
[warning] 60-60:
"Maping" should be "Mapping".crates/mako/src/visitors/dep_replacer.rs (1)
254-254: 确保测试用例覆盖新的数据结构在测试中引入
ResolvedReplaceInfo，请确保测试用例充分覆盖新数据结构的使用场景，以验证其功能是否符合预期。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
crates/mako/src/compiler.rs(1 hunks)crates/mako/src/config/experimental.rs(1 hunks)crates/mako/src/config/mako.config.default.json(1 hunks)crates/mako/src/generate/transform.rs(3 hunks)crates/mako/src/plugins.rs(1 hunks)crates/mako/src/plugins/bundless_compiler.rs(2 hunks)crates/mako/src/plugins/central_ensure.rs(1 hunks)crates/mako/src/visitors/dep_replacer.rs(6 hunks)crates/mako/src/visitors/dynamic_import.rs(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/plugins.rs
🧰 Additional context used
🪛 GitHub Check: codecov/patch
crates/mako/src/compiler.rs
[warning] 302-302: crates/mako/src/compiler.rs#L302
Added line #L302 was not covered by tests
crates/mako/src/generate/transform.rs
[warning] 91-94: crates/mako/src/generate/transform.rs#L91-L94
Added lines #L91 - L94 were not covered by tests
[warning] 96-98: crates/mako/src/generate/transform.rs#L96-L98
Added lines #L96 - L98 were not covered by tests
crates/mako/src/plugins/bundless_compiler.rs
[warning] 83-87: crates/mako/src/plugins/bundless_compiler.rs#L83-L87
Added lines #L83 - L87 were not covered by tests
crates/mako/src/plugins/central_ensure.rs
[warning] 13-13: crates/mako/src/plugins/central_ensure.rs#L13
Added line #L13 was not covered by tests
[warning] 15-17: crates/mako/src/plugins/central_ensure.rs#L15-L17
Added lines #L15 - L17 were not covered by tests
[warning] 20-21: crates/mako/src/plugins/central_ensure.rs#L20-L21
Added lines #L20 - L21 were not covered by tests
[warning] 24-24: crates/mako/src/plugins/central_ensure.rs#L24
Added line #L24 was not covered by tests
[warning] 26-26: crates/mako/src/plugins/central_ensure.rs#L26
Added line #L26 was not covered by tests
[warning] 28-30: crates/mako/src/plugins/central_ensure.rs#L28-L30
Added lines #L28 - L30 were not covered by tests
[warning] 32-32: crates/mako/src/plugins/central_ensure.rs#L32
Added line #L32 was not covered by tests
[warning] 34-35: crates/mako/src/plugins/central_ensure.rs#L34-L35
Added lines #L34 - L35 were not covered by tests
[warning] 37-39: crates/mako/src/plugins/central_ensure.rs#L37-L39
Added lines #L37 - L39 were not covered by tests
[warning] 41-42: crates/mako/src/plugins/central_ensure.rs#L41-L42
Added lines #L41 - L42 were not covered by tests
[warning] 44-44: crates/mako/src/plugins/central_ensure.rs#L44
Added line #L44 was not covered by tests
[warning] 46-48: crates/mako/src/plugins/central_ensure.rs#L46-L48
Added lines #L46 - L48 were not covered by tests
[warning] 50-50: crates/mako/src/plugins/central_ensure.rs#L50
Added line #L50 was not covered by tests
[warning] 53-53: crates/mako/src/plugins/central_ensure.rs#L53
Added line #L53 was not covered by tests
[warning] 55-55: crates/mako/src/plugins/central_ensure.rs#L55
Added line #L55 was not covered by tests
[warning] 74-75: crates/mako/src/plugins/central_ensure.rs#L74-L75
Added lines #L74 - L75 were not covered by tests
crates/mako/src/visitors/dep_replacer.rs
[warning] 26-26: crates/mako/src/visitors/dep_replacer.rs#L26
Added line #L26 was not covered by tests
[warning] 28-29: crates/mako/src/visitors/dep_replacer.rs#L28-L29
Added lines #L28 - L29 were not covered by tests
[warning] 36-36: crates/mako/src/visitors/dep_replacer.rs#L36
Added line #L36 was not covered by tests
crates/mako/src/visitors/dynamic_import.rs
[warning] 117-117: crates/mako/src/visitors/dynamic_import.rs#L117
Added line #L117 was not covered by tests
[warning] 119-119: crates/mako/src/visitors/dynamic_import.rs#L119
Added line #L119 was not covered by tests
[warning] 150-153: crates/mako/src/visitors/dynamic_import.rs#L150-L153
Added lines #L150 - L153 were not covered by tests
[warning] 179-179: crates/mako/src/visitors/dynamic_import.rs#L179
Added line #L179 was not covered by tests
[warning] 185-185: crates/mako/src/visitors/dynamic_import.rs#L185
Added line #L185 was not covered by tests
🪛 GitHub Check: Spell Check
crates/mako/src/plugins/central_ensure.rs
[warning] 59-59:
"Maping" should be "Mapping".
[warning] 60-60:
"Maping" should be "Mapping".
🔇 Additional comments (11)
crates/mako/src/config/experimental.rs (1)
13-13: 字段添加正确且符合结构体的序列化规则！
新增的 central_ensure 字段遵循了现有的代码规范，包括：
- 正确的可见性修饰符（pub）
- 符合 serde 的驼峰命名转换规则
crates/mako/src/config/mako.config.default.json (1)
74-75: 配置更改已确认
新增的实验性配置项 centralEnsure 正确放置在 experimental 部分，默认值设置为 true 符合开发环境测试的需求。
运行以下脚本验证配置模式是否已更新：
✅ Verification successful
Let me search for the schema definition to verify if the new configuration option is properly defined.
Let me try a different search pattern to find the schema definition.
Let me check the content of the ExperimentalConfig struct since we found it in the previous search.
配置模式已正确更新
ExperimentalConfig 结构体中已包含 central_ensure 字段，使用了正确的驼峰命名法序列化配置，并且类型定义为 bool。配置模式与默认配置文件保持一致。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证配置模式是否包含新的 centralEnsure 选项
# 测试：搜索配置模式定义
rg -A 5 "experimental.*struct|centralEnsure" "crates/mako/src/config/"
Length of output: 572
Script:
#!/bin/bash
# Search for schema definitions in Rust files
rg -t rust "experimental.*struct|centralEnsure" "crates/mako/src/config/"
# Search for any documentation or type definitions related to centralEnsure
ast-grep --pattern 'struct $_ {
$$$
central_ensure: $_,
$$$
}'
Length of output: 144
Script:
#!/bin/bash
# Search for experimental configuration structs
rg -t rust "struct.*Experimental|struct.*Config" "crates/mako/src/"
# Also search for any serialization attributes that might define the field
rg -t rust "#\[.*serde.*\].*central.*ensure" "crates/mako/src/"
Length of output: 1623
Script:
#!/bin/bash
# Check the content of experimental config struct
cat crates/mako/src/config/experimental.rs
Length of output: 739
crates/mako/src/visitors/dynamic_import.rs (1)
116-122: 需要补充测试用例
新增的 central_ensure 条件分支缺少测试覆盖。建议添加以下场景的测试用例：
- watch 模式下的 central_ensure 功能
- 非 watch 模式下的行为
- experimental.central_ensure 配置的影响
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 117-117: crates/mako/src/visitors/dynamic_import.rs#L117
Added line #L117 was not covered by tests
[warning] 119-119: crates/mako/src/visitors/dynamic_import.rs#L119
Added line #L119 was not covered by tests
crates/mako/src/plugins/bundless_compiler.rs (2)
27-27: 导入声明看起来没有问题！
新增的 ResolvedReplaceInfo 导入与现有的依赖替换相关导入放在一起，结构清晰。
83-89: 建议为新的数据结构添加单元测试
代码重构使用 ResolvedReplaceInfo 结构体替代了之前的元组，提高了代码的可读性和类型安全性。但是静态分析显示这部分代码缺少测试覆盖。
建议添加以下测试场景：
- 正常路径解析
- 相对路径转换
- 边界情况处理
需要我帮您生成测试用例吗？我可以提供一个完整的测试模块实现。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 83-87: crates/mako/src/plugins/bundless_compiler.rs#L83-L87
Added lines #L83 - L87 were not covered by tests
crates/mako/src/generate/transform.rs (2)
30-30: 导入更改看起来不错！
新增的 ResolvedReplaceInfo 导入与代码重构的方向一致。
87-107: 建议增加对 Worker 解析逻辑的测试覆盖
代码逻辑本身看起来是合理的，但静态分析显示 Worker 相关的代码路径缺少测试覆盖。建议添加相关测试用例以确保功能的正确性。
需要我帮助编写测试用例吗？我可以协助生成以下场景的测试：
- Worker 带有自定义 chunk 名称的情况
- Worker 使用默认 chunk 名称的情况
- 非 Worker 类型的常规解析情况
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 91-94: crates/mako/src/generate/transform.rs#L91-L94
Added lines #L91 - L94 were not covered by tests
[warning] 96-98: crates/mako/src/generate/transform.rs#L96-L98
Added lines #L96 - L98 were not covered by tests
crates/mako/src/compiler.rs (1)
301-304: 插件添加实现正确，建议验证配置项
插件的条件添加逻辑符合实验性功能的处理方式。建议验证实验性配置项在配置文件中的默认值设置。
运行以下脚本验证配置项：
✅ Verification successful
实验性配置项 central_ensure 已正确设置
配置项 central_ensure 在默认配置文件中已设置为 true，且在配置结构体中已正确定义为布尔类型。此配置项的实现完全符合预期。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 central_ensure 配置项的默认值设置
# 测试：查找配置文件中的默认值设置
rg -l "central_ensure|centralEnsure"
Length of output: 223
Script:
#!/bin/bash
# Check default configuration value in default config file
cat crates/mako/src/config/mako.config.default.json
# Check experimental config struct definition
cat crates/mako/src/config/experimental.rs
Length of output: 2866
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 302-302: crates/mako/src/compiler.rs#L302
Added line #L302 was not covered by tests
crates/mako/src/visitors/dep_replacer.rs (3)
201-201: 代码修改符合预期
此修改正确地使用了新的 ResolvedReplaceInfo 结构体，与更新后的数据结构保持一致。
370-370: 更新 run 函数签名以匹配新的参数类型
run 函数的参数 resolved 类型更新为 HashMap<String, ResolvedReplaceInfo>，修改符合预期，请确保相关调用和逻辑也已同步更新。
36-36: 🛠️ Refactor suggestion
确保对 resolved 字段类型修改的全面测试
DependenciesToReplace 结构体中 resolved 字段的类型已更改为 HashMap<String, ResolvedReplaceInfo>。请确保所有使用该字段的逻辑都已更新，并且添加相应的单元测试来验证新的数据结构。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 36-36: crates/mako/src/visitors/dep_replacer.rs#L36
Added line #L36 was not covered by tests
| #[derive(Debug, Clone)] | ||
| pub struct ResolvedReplaceInfo { | ||
| pub to_replace_source: String, | ||
| pub resolved_module_id: ModuleId, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议为新结构体 ResolvedReplaceInfo 添加单元测试
新添加的结构体 ResolvedReplaceInfo 未被测试覆盖，建议添加相应的单元测试以确保其功能的正确性和稳定性。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 26-26: crates/mako/src/visitors/dep_replacer.rs#L26
Added line #L26 was not covered by tests
[warning] 28-29: crates/mako/src/visitors/dep_replacer.rs#L28-L29
Added lines #L28 - L29 were not covered by tests
| if let Some(replace_info) = self.to_replace.resolved.get(&source_string) { | ||
| let (path, _search, query, _) = | ||
| parse_path(&replace_info.resolved_module_id.id).unwrap(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议处理 parse_path 的可能错误以避免 panic
在第 132 行，使用了 parse_path(&replace_info.resolved_module_id.id).unwrap();，如果 parse_path 返回 None，unwrap() 会导致程序 panic。为了提高代码的健壮性，建议处理可能的错误。
建议修改为：
- let (path, _search, query, _) = parse_path(&replace_info.resolved_module_id.id).unwrap();
+ if let Some((path, _search, query, _)) = parse_path(&replace_info.resolved_module_id.id) {
+ // 现有逻辑
+ } else {
+ // 适当地处理错误，例如记录日志或返回错误
+ }Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
crates/mako/src/plugins/central_ensure.rs (4)
13-20: 建议改进错误消息的可读性当前的错误消息格式可以更加清晰：
- .map_err(|e| anyhow!("Read_Module_Graph_error:\n{:?}", e))?; + .map_err(|e| anyhow!("无法读取模块图：{}", e))?; - .map_err(|e| anyhow!("Read_Chunk_Graph_error:\n{:?}", e))?; + .map_err(|e| anyhow!("无法读取分块图：{}", e))?;🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-17: crates/mako/src/plugins/central_ensure.rs#L16-L17
Added lines #L16 - L17 were not covered by tests
[warning] 20-20: crates/mako/src/plugins/central_ensure.rs#L20
Added line #L20 was not covered by tests
55-55: 需要跟踪压缩优化的 TODO代码中的 TODO 注释提到了需要压缩映射以减少重复的 chunk IDs。这是一个重要的性能优化点。
是否需要我创建一个 GitHub issue 来跟踪这个优化任务？
59-74: JavaScript 代码需要改进命名一致性
- 变量命名应该使用驼峰式：
- let map = {ensure_map}; + let ensureMap = {ensure_map};
- 函数命名应保持一致性：
- requireModule.updateEnsure2Map = function(newMapping) { - map = newMapping; + requireModule.updateEnsure2Map = function(newEnsureMap) { + ensureMap = newEnsureMap;
- 参数命名应更具描述性：
- requireModule.ensure2 = function(chunkId){ + requireModule.ensure2 = function(moduleId){
1-79: 建议考虑可测试性和错误处理的架构改进
可测试性：
- 考虑将
module_ensure_map的核心逻辑抽离为更小的纯函数，便于单元测试- 添加集成测试验证整个插件的功能
错误处理：
- 考虑定义自定义错误类型，而不是直接使用
anyhow- 为不同类型的错误提供更详细的上下文信息
性能优化：
- 实现 TODO 中提到的映射压缩功能
- 考虑添加缓存机制，避免重复计算
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-13: crates/mako/src/plugins/central_ensure.rs#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 16-17: crates/mako/src/plugins/central_ensure.rs#L16-L17
Added lines #L16 - L17 were not covered by tests
[warning] 20-20: crates/mako/src/plugins/central_ensure.rs#L20
Added line #L20 was not covered by tests
[warning] 22-22: crates/mako/src/plugins/central_ensure.rs#L22
Added line #L22 was not covered by tests
[warning] 24-26: crates/mako/src/plugins/central_ensure.rs#L24-L26
Added lines #L24 - L26 were not covered by tests
[warning] 28-28: crates/mako/src/plugins/central_ensure.rs#L28
Added line #L28 was not covered by tests
[warning] 30-31: crates/mako/src/plugins/central_ensure.rs#L30-L31
Added lines #L30 - L31 were not covered by tests
[warning] 33-35: crates/mako/src/plugins/central_ensure.rs#L33-L35
Added lines #L33 - L35 were not covered by tests
[warning] 37-38: crates/mako/src/plugins/central_ensure.rs#L37-L38
Added lines #L37 - L38 were not covered by tests
[warning] 40-41: crates/mako/src/plugins/central_ensure.rs#L40-L41
Added lines #L40 - L41 were not covered by tests
[warning] 43-43: crates/mako/src/plugins/central_ensure.rs#L43
Added line #L43 was not covered by tests
[warning] 45-46: crates/mako/src/plugins/central_ensure.rs#L45-L46
Added lines #L45 - L46 were not covered by tests
[warning] 49-49: crates/mako/src/plugins/central_ensure.rs#L49
Added line #L49 was not covered by tests
[warning] 51-53: crates/mako/src/plugins/central_ensure.rs#L51-L53
Added lines #L51 - L53 were not covered by tests
[warning] 56-56: crates/mako/src/plugins/central_ensure.rs#L56
Added line #L56 was not covered by tests
[warning] 58-58: crates/mako/src/plugins/central_ensure.rs#L58
Added line #L58 was not covered by tests
[warning] 77-78: crates/mako/src/plugins/central_ensure.rs#L77-L78
Added lines #L77 - L78 were not covered by testscrates/mako/src/generate/hmr.rs (1)
27-27: 建议为新增代码添加测试覆盖第27、29-31、33、35-36、38-40行的新代码未被测试覆盖。为了确保代码的可靠性和稳定性，建议为这些部分添加相应的测试。
您是否需要协助编写这些新代码段的测试用例？
Also applies to: 29-31, 33-33, 35-36, 38-40
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 27-27: crates/mako/src/generate/hmr.rs#L27
Added line #L27 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
crates/mako/src/generate/hmr.rs(2 hunks)crates/mako/src/plugins/central_ensure.rs(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
crates/mako/src/generate/hmr.rs
[warning] 27-27: crates/mako/src/generate/hmr.rs#L27
Added line #L27 was not covered by tests
[warning] 29-31: crates/mako/src/generate/hmr.rs#L29-L31
Added lines #L29 - L31 were not covered by tests
[warning] 33-33: crates/mako/src/generate/hmr.rs#L33
Added line #L33 was not covered by tests
[warning] 35-36: crates/mako/src/generate/hmr.rs#L35-L36
Added lines #L35 - L36 were not covered by tests
[warning] 38-40: crates/mako/src/generate/hmr.rs#L38-L40
Added lines #L38 - L40 were not covered by tests
crates/mako/src/plugins/central_ensure.rs
[warning] 12-13: crates/mako/src/plugins/central_ensure.rs#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 16-17: crates/mako/src/plugins/central_ensure.rs#L16-L17
Added lines #L16 - L17 were not covered by tests
[warning] 20-20: crates/mako/src/plugins/central_ensure.rs#L20
Added line #L20 was not covered by tests
[warning] 22-22: crates/mako/src/plugins/central_ensure.rs#L22
Added line #L22 was not covered by tests
[warning] 24-26: crates/mako/src/plugins/central_ensure.rs#L24-L26
Added lines #L24 - L26 were not covered by tests
[warning] 28-28: crates/mako/src/plugins/central_ensure.rs#L28
Added line #L28 was not covered by tests
[warning] 30-31: crates/mako/src/plugins/central_ensure.rs#L30-L31
Added lines #L30 - L31 were not covered by tests
[warning] 33-35: crates/mako/src/plugins/central_ensure.rs#L33-L35
Added lines #L33 - L35 were not covered by tests
[warning] 37-38: crates/mako/src/plugins/central_ensure.rs#L37-L38
Added lines #L37 - L38 were not covered by tests
[warning] 40-41: crates/mako/src/plugins/central_ensure.rs#L40-L41
Added lines #L40 - L41 were not covered by tests
[warning] 43-43: crates/mako/src/plugins/central_ensure.rs#L43
Added line #L43 was not covered by tests
[warning] 45-46: crates/mako/src/plugins/central_ensure.rs#L45-L46
Added lines #L45 - L46 were not covered by tests
[warning] 49-49: crates/mako/src/plugins/central_ensure.rs#L49
Added line #L49 was not covered by tests
[warning] 51-53: crates/mako/src/plugins/central_ensure.rs#L51-L53
Added lines #L51 - L53 were not covered by tests
[warning] 56-56: crates/mako/src/plugins/central_ensure.rs#L56
Added line #L56 was not covered by tests
[warning] 58-58: crates/mako/src/plugins/central_ensure.rs#L58
Added line #L58 was not covered by tests
[warning] 77-78: crates/mako/src/plugins/central_ensure.rs#L77-L78
Added lines #L77 - L78 were not covered by tests
🔇 Additional comments (2)
crates/mako/src/plugins/central_ensure.rs (1)
1-10: 代码结构清晰，导入合理！
导入的模块和结构体定义符合插件系统的要求。
crates/mako/src/generate/hmr.rs (1)
13-13: 导入 module_ensure_map 模块
导入中央确保映射模块以支持新的功能，看起来正确且必要。
| pub fn module_ensure_map(context: &Arc<Context>) -> anyhow::Result<BTreeMap<String, Vec<String>>> { | ||
| let mg = context | ||
| .module_graph | ||
| .read() | ||
| .map_err(|e| anyhow!("Read_Module_Graph_error:\n{:?}", e))?; | ||
| let cg = context | ||
| .chunk_graph | ||
| .read() | ||
| .map_err(|e| anyhow!("Read_Chunk_Graph_error:\n{:?}", e))?; | ||
|
|
||
| let mut chunk_async_map: BTreeMap<String, Vec<String>> = Default::default(); | ||
|
|
||
| mg.modules().iter().for_each(|module| { | ||
| let be_dynamic_imported = mg | ||
| .get_dependents(&module.id) | ||
| .iter() | ||
| .any(|(_, dep)| dep.resolve_type.is_dynamic_esm()); | ||
|
|
||
| if be_dynamic_imported { | ||
| cg.get_chunk_for_module(&module.id) | ||
| .iter() | ||
| .for_each(|chunk| { | ||
| let deps_chunks = cg | ||
| .installable_descendants_chunk(&chunk.id) | ||
| .iter() | ||
| .map(|chunk_id| chunk_id.generate(context)) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| chunk_async_map.insert(generate_module_id(&module.id.id, context), deps_chunks); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| Ok(chunk_async_map) | ||
| } |
There was a problem hiding this comment.
需要添加单元测试覆盖关键功能
该函数实现了模块依赖关系的核心映射逻辑，但目前缺少测试覆盖。建议添加以下测试场景：
- 正常情况下的模块映射
- 读取图结构失败的错误处理
- 动态导入模块的处理
- 空依赖的边界情况
需要我帮您生成测试用例代码吗？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 12-13: crates/mako/src/plugins/central_ensure.rs#L12-L13
Added lines #L12 - L13 were not covered by tests
[warning] 16-17: crates/mako/src/plugins/central_ensure.rs#L16-L17
Added lines #L16 - L17 were not covered by tests
[warning] 20-20: crates/mako/src/plugins/central_ensure.rs#L20
Added line #L20 was not covered by tests
[warning] 22-22: crates/mako/src/plugins/central_ensure.rs#L22
Added line #L22 was not covered by tests
[warning] 24-26: crates/mako/src/plugins/central_ensure.rs#L24-L26
Added lines #L24 - L26 were not covered by tests
[warning] 28-28: crates/mako/src/plugins/central_ensure.rs#L28
Added line #L28 was not covered by tests
[warning] 30-31: crates/mako/src/plugins/central_ensure.rs#L30-L31
Added lines #L30 - L31 were not covered by tests
[warning] 33-35: crates/mako/src/plugins/central_ensure.rs#L33-L35
Added lines #L33 - L35 were not covered by tests
[warning] 37-38: crates/mako/src/plugins/central_ensure.rs#L37-L38
Added lines #L37 - L38 were not covered by tests
[warning] 40-41: crates/mako/src/plugins/central_ensure.rs#L40-L41
Added lines #L40 - L41 were not covered by tests
[warning] 43-43: crates/mako/src/plugins/central_ensure.rs#L43
Added line #L43 was not covered by tests
[warning] 45-46: crates/mako/src/plugins/central_ensure.rs#L45-L46
Added lines #L45 - L46 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
crates/mako/src/plugins/central_ensure.rs (3)
12-46: 需要为公共函数添加文档注释作为一个公共 API，建议添加详细的文档注释，包括：
- 函数的主要功能
- 参数说明
- 返回值说明
- 可能的错误情况
- 使用示例
建议添加如下文档注释：
+/// 生成模块ID到其对应的异步chunk ID的映射关系 +/// +/// # Arguments +/// * `context` - 编译上下文，包含模块图和chunk图信息 +/// +/// # Returns +/// 返回一个BTreeMap，key为模块ID，value为该模块依赖的所有chunk ID列表 +/// +/// # Errors +/// 当无法获取模块图或chunk图的读锁时会返回错误
24-43: 性能优化建议当前实现中存在多层嵌套迭代，可能会影响性能。建议：
- 考虑使用
par_iter()进行并行处理- 提前预分配
chunk_async_map的容量- 将复杂的嵌套逻辑拆分为独立的函数
55-55: 需要解决 TODO 注释关于压缩映射以减少重复的 chunk ID 的 TODO 注释需要被处理。这对于减少运行时的内存占用很重要。
我可以帮助实现一个压缩算法来减少重复的 chunk ID。需要我提供具体实现吗？
crates/mako/src/generate/chunk_graph.rs (1)
66-70: 代码实现清晰且符合预期！该方法实现了异步块的查找功能，与 PR 中提到的 ensure2 运行时 API 的需求相匹配。实现采用了 Rust 的惯用模式，代码简洁明了。
建议考虑以下几点：
- 该方法将成为 central ensure 特性的核心组件，建议添加详细的文档注释说明其用途和使用场景
- 可以考虑添加单元测试以验证异步块查找的正确性
这个新方法是支持将模块映射关系集中到入口块的关键组件。它将帮助简化动态导入的实现，使代码生成更加稳定。不过需要注意，正如 PR 描述中提到的，这种改变会增加入口块的大小，建议在开发环境中先进行充分测试。
crates/mako/src/visitors/dynamic_import.rs (2)
50-59: 建议简化 interop_replace 的获取逻辑当前代码在获取 interop_replace 时使用了硬编码的字符串键值，建议将其提取为常量以提高可维护性。
建议修改如下:
+ const INTEROP_HELPER_KEY: &str = "@swc/helpers/_/_interop_require_wildcard"; - let interop_replace = self - .dep_to_replace - .resolved - .get("@swc/helpers/_/_interop_require_wildcard") - .unwrap(); + let interop_replace = self + .dep_to_replace + .resolved + .get(INTEROP_HELPER_KEY) + .unwrap();
250-259: 建议增加更多测试用例当前测试数据结构已更新，但建议添加以下测试场景：
- central_ensure 功能的测试
- 错误处理的测试
- 不同 chunk 配置的测试
建议添加如下测试用例：
#[test] fn test_central_ensure() { // 测试启用 central_ensure 时的行为 } #[test] fn test_error_handling() { // 测试各种错误场景 } #[test] fn test_different_chunk_configs() { // 测试不同的 chunk 配置 }crates/mako/src/plugins/bundless_compiler.rs (1)
83-90: 结构体的使用提高了代码的可读性和可维护性！使用
ResolvedReplaceInfo结构体替代元组是个很好的改进：
- 通过命名字段提高了代码的可读性
- 使数据结构更加清晰和类型安全
- 为未来可能的扩展提供了更好的基础
建议考虑添加结构体字段的文档注释，以便更好地理解每个字段的用途。
ResolvedReplaceInfo { + /// 模块所属的 chunk ID，用于代码分割 chunk_id: None, + /// 替换后的模块源路径 to_replace_source: replacement, + /// 解析后的模块 ID resolved_module_id: id.clone(), }crates/mako/src/generate/transform.rs (1)
87-125: 代码重构改进了依赖解析的结构化处理！重构后的代码通过使用
ResolvedReplaceInfo结构体提高了类型安全性，并清晰地处理了不同的解析类型（Worker、DynamicImport 等）。建议考虑以下优化：
- let chunk_id = match import_options.get_chunk_name() { - Some(chunk_name) => generate_module_id(chunk_name, &context), - None => id.generate(&context), - }; + let chunk_id = import_options + .get_chunk_name() + .map_or_else(|| id.generate(&context), |chunk_name| generate_module_id(chunk_name, &context));这样可以使代码更简洁，同时保持相同的功能。
crates/mako/src/visitors/dep_replacer.rs (2)
26-31: 结构体定义清晰，建议添加文档注释结构体的设计合理，字段命名清晰。建议为结构体及其字段添加文档注释，以便其他开发者更好地理解其用途。
建议添加如下文档注释：
+/// 封装模块解析信息的结构体 #[derive(Debug, Clone)] pub struct ResolvedReplaceInfo { + /// 可选的 chunk ID pub chunk_id: Option<String>, + /// 要替换的源代码路径 pub to_replace_source: String, + /// 已解析的模块 ID pub resolved_module_id: ModuleId, }
37-37: 注释需要更新以反映新的数据结构原注释仍然描述了旧的元组结构
(generate_id, raw_module_id)，需要更新以反映新的ResolvedReplaceInfo结构。建议更新注释为：
- // resolved stores the "source" maps to (generate_id, raw_module_id) - // e.g. "react" => ("hashed_id", "/abs/to/react/index.js") + // resolved stores the "source" maps to ResolvedReplaceInfo + // e.g. "react" => ResolvedReplaceInfo { chunk_id, to_replace_source, resolved_module_id } pub resolved: HashMap<String, ResolvedReplaceInfo>,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
crates/mako/src/generate/chunk_graph.rs(1 hunks)crates/mako/src/generate/transform.rs(3 hunks)crates/mako/src/plugins/bundless_compiler.rs(2 hunks)crates/mako/src/plugins/central_ensure.rs(1 hunks)crates/mako/src/visitors/dep_replacer.rs(6 hunks)crates/mako/src/visitors/dynamic_import.rs(6 hunks)
🔇 Additional comments (10)
crates/mako/src/plugins/central_ensure.rs (2)
1-79: 需要添加单元测试
这个模块实现了关键的运行时功能，但缺少测试覆盖。建议添加以下测试场景：
- 正常情况下的模块映射生成
- 错误处理测试（读锁获取失败等）
- 边界情况测试（空依赖、循环依赖等）
- JavaScript 运行时代码的功能测试
需要我帮您生成测试用例代码吗？
59-74: 🛠️ Refactor suggestion
JavaScript 运行时代码需要改进
当前的 JavaScript 实现存在以下问题：
- 拼写错误：
updateEnsure2Map而不是updateEnsuire2Map - 缺少错误处理：当
map[chunkId]不存在时应该提供更好的错误信息 - 代码格式化不一致
建议修改为：
(function() {
let map = {ensure_map};
- requireModule.updateEnsure2Map = function(newMapping) {
+ requireModule.updateEnsure2Map = function(newMapping) {
map = newMapping;
}
requireModule.ensure2 = function(chunkId) {
+ if (typeof chunkId !== 'string') {
+ return Promise.reject(new Error('chunkId must be a string'));
+ }
let toEnsure = map[chunkId];
if (toEnsure) {
return Promise.all(toEnsure.map(function(c) {
return requireModule.ensure(c);
}));
} else {
+ console.warn(`No chunks found for module: ${chunkId}`);
return Promise.resolve();
}
};
})();crates/mako/src/visitors/dynamic_import.rs (2)
12-16: 导入声明看起来合理且完整！
所有必需的依赖项都已正确导入，包括新添加的 ResolvedReplaceInfo 类型。
148-154: central_ensure 方法实现简洁明了
方法实现符合单一职责原则，代码清晰易懂。
crates/mako/src/plugins/bundless_compiler.rs (3)
27-27: 导入声明看起来没有问题！
新增的 ResolvedReplaceInfo 结构体导入与现有的依赖替换相关导入放在一起，符合代码组织规范。
94-95: 类型声明修改合理！
将 HashMap 的值类型从元组更新为 ResolvedReplaceInfo 结构体，保持了类型一致性，同时提高了代码的可维护性。
83-95: 建议验证相关文件的改动
为确保 ensure runtime 迁移的完整性，建议验证以下几点：
- 检查是否还有其他文件使用旧的元组形式
- 确认 chunk_id 相关的逻辑是否已完全迁移
✅ Verification successful
Let me gather more context about the provider.rs file to understand if the tuple usage there is related to our changes.
代码改动验证结果
经过验证，发现以下情况：
-
provider.rs中的元组类型HashMap<String, (String, String)>与当前改动无关，它用于处理 Node.js 内置模块的映射，不涉及 chunk 或 runtime 逻辑。 -
ResolvedReplaceInfo结构的改动已正确应用于所有相关文件：- bundless_compiler.rs
- transform.rs
- dynamic_import.rs
- dep_replacer.rs
-
ensure runtime 相关代码已完成迁移，从
ensure到ensure2的转换也已实现。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索可能遗漏的元组用法
echo "检查可能遗漏的元组用法..."
rg "HashMap<String,\s*\(String,\s*String\)>" ./crates/mako/src/
# 搜索 ensure runtime 相关代码
echo "检查 ensure runtime 相关代码..."
rg "__mako_require__\.ensure" ./crates/mako/src/
Length of output: 994
Script:
#!/bin/bash
# Check the context of Providers type usage
echo "检查 Providers 类型的使用上下文..."
rg -B 2 -A 2 "type Providers = " ./crates/mako/src/
# Check if provider.rs has any relation to runtime/chunks
echo "检查 provider.rs 与 runtime/chunks 的关联..."
rg "chunk|runtime" ./crates/mako/src/config/provider.rs
# Check the usage of ResolvedReplaceInfo
echo "检查 ResolvedReplaceInfo 的使用..."
rg "ResolvedReplaceInfo" ./crates/mako/src/
Length of output: 2889
crates/mako/src/generate/transform.rs (1)
30-30: 导入声明看起来没有问题！
新增的 ResolvedReplaceInfo 导入与代码重构的目标相符，有助于提高代码的类型安全性。
crates/mako/src/visitors/dep_replacer.rs (2)
202-202: 代码简化得当
将元组解构替换为直接使用 to_replace_source 使代码更加清晰。
131-133:
建议处理 parse_path 的错误情况
直接使用 unwrap() 可能在 parse_path 返回 None 时导致程序崩溃。
建议使用 if let 或 match 来优雅地处理错误情况：
- let (path, _search, query, _) = parse_path(&replace_info.resolved_module_id.id).unwrap();
+ let (path, _search, query, _) = match parse_path(&replace_info.resolved_module_id.id) {
+ Some(parsed) => parsed,
+ None => return false,
+ };Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
crates/mako/src/plugins/central_ensure.rs (2)
15-20: 改进错误消息的可读性当前错误消息使用了下划线分隔和大写字母，建议改为更符合 Rust 习惯的格式：
- .map_err(|e| anyhow!("Read_Module_Graph_error:\n{:?}", e))?; + .map_err(|e| anyhow!("无法读取模块图: {}", e))?; - .map_err(|e| anyhow!("Read_Chunk_Graph_error:\n{:?}", e))?; + .map_err(|e| anyhow!("无法读取分块图: {}", e))?;
55-55: 请处理压缩相关的 TODO 注释代码中的 TODO 提到需要压缩映射以减少重复的 chunk IDs。这是一个重要的性能优化点，建议在合并前完成此优化。
需要我帮您设计压缩算法吗？
crates/mako/src/plugin.rs (1)
375-381: HMR 运行时更新代码聚合实现完善实现逻辑清晰，与
runtime_plugins_code保持一致的处理方式。建议考虑以下几点：
- 在大量插件的情况下，使用
String拼接可能会有性能影响- 考虑添加注释说明更新代码的预期格式
建议考虑如下优化：
pub fn hmr_runtime_update_code(&self, context: &Arc<Context>) -> Result<String> { let mut plugins = Vec::new(); for plugin in &self.plugins { plugins.extend(plugin.hmr_runtime_updates(context)?); } - Ok(plugins.join("\n")) + // 预分配容量以提高性能 + let capacity = plugins.iter().map(|s| s.len() + 1).sum(); + let mut result = String::with_capacity(capacity); + for (i, plugin) in plugins.iter().enumerate() { + if i > 0 { + result.push('\n'); + } + result.push_str(plugin); + } + Ok(result)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
crates/mako/src/generate/hmr.rs(1 hunks)crates/mako/src/plugin.rs(2 hunks)crates/mako/src/plugins/central_ensure.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/mako/src/generate/hmr.rs
🔇 Additional comments (3)
crates/mako/src/plugins/central_ensure.rs (2)
1-10: 导入和结构定义看起来合适！
导入的模块和类型都是必要的，结构体定义简洁明了。
48-90: 插件实现总体良好！
Plugin trait 的实现逻辑清晰，错误处理得当。特别是 HMR 更新的处理非常完整。
crates/mako/src/plugin.rs (1)
162-164: 新增的 HMR 运行时更新方法实现合理
这个默认实现遵循了 Rust 的 trait 设计模式，与现有的 runtime_plugins 方法保持一致的风格。空向量作为默认返回值是合适的选择，允许插件按需覆盖此方法。
| pub fn module_ensure_map(context: &Arc<Context>) -> anyhow::Result<BTreeMap<String, Vec<String>>> { | ||
| let mg = context | ||
| .module_graph | ||
| .read() | ||
| .map_err(|e| anyhow!("Read_Module_Graph_error:\n{:?}", e))?; | ||
| let cg = context | ||
| .chunk_graph | ||
| .read() | ||
| .map_err(|e| anyhow!("Read_Chunk_Graph_error:\n{:?}", e))?; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议优化锁的使用方式
当前代码连续获取两个读锁可能导致性能问题或死锁。建议合并锁的获取操作：
let (mg, cg) = {
let (mg_guard, cg_guard) = try_join!(
context.module_graph.try_read_timeout(Duration::from_secs(1)),
context.chunk_graph.try_read_timeout(Duration::from_secs(1))
).map_err(|e| anyhow!("获取图读锁失败: {}", e))?;
(mg_guard, cg_guard)
};| pub fn module_ensure_map(context: &Arc<Context>) -> anyhow::Result<BTreeMap<String, Vec<String>>> { | ||
| let mg = context | ||
| .module_graph | ||
| .read() | ||
| .map_err(|e| anyhow!("Read_Module_Graph_error:\n{:?}", e))?; | ||
| let cg = context | ||
| .chunk_graph | ||
| .read() | ||
| .map_err(|e| anyhow!("Read_Chunk_Graph_error:\n{:?}", e))?; | ||
|
|
||
| let mut chunk_async_map: BTreeMap<String, Vec<String>> = Default::default(); | ||
|
|
||
| mg.modules().iter().for_each(|module| { | ||
| let be_dynamic_imported = mg | ||
| .get_dependents(&module.id) | ||
| .iter() | ||
| .any(|(_, dep)| dep.resolve_type.is_dynamic_esm()); | ||
|
|
||
| if be_dynamic_imported { | ||
| cg.get_async_chunk_for_module(&module.id) | ||
| .iter() | ||
| .for_each(|chunk| { | ||
| let deps_chunks = cg | ||
| .installable_descendants_chunk(&chunk.id) | ||
| .iter() | ||
| .map(|chunk_id| chunk_id.generate(context)) | ||
| .collect::<Vec<_>>(); | ||
|
|
||
| chunk_async_map.insert(generate_module_id(&module.id.id, context), deps_chunks); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| Ok(chunk_async_map) | ||
| } |
There was a problem hiding this comment.
需要添加单元测试
module_ensure_map 函数实现了核心的映射逻辑，但缺少测试覆盖。建议添加以下测试场景：
- 正常情况下的模块映射
- 读取图结构失败的错误处理
- 动态导入模块的处理
- 空依赖的边界情况
需要我帮您生成测试用例代码吗？
| r#" | ||
| (function(){{ | ||
| let map = {ensure_map}; | ||
| requireModule.updateEnsure2Map = function(newMapping) {{ | ||
| map = newMapping; | ||
| }} | ||
| requireModule.ensure2 = function(chunkId){{ | ||
| let toEnsure = map[chunkId]; | ||
| if (toEnsure) {{ | ||
| return Promise.all(toEnsure.map(function(c){{ return requireModule.ensure(c); }})) | ||
| }}else{{ | ||
| return Promise.resolve(); | ||
| }} | ||
| }}; | ||
| }})(); | ||
| "# |
There was a problem hiding this comment.
🛠️ Refactor suggestion
优化 JavaScript 代码质量
JavaScript 代码有以下需要改进的地方：
- 拼写错误：
updateEnsure2Map - 缺少严格模式声明
- 可以使用更现代的语法
建议修改如下：
r#"
+'use strict';
(function(){{
let map = {ensure_map};
- requireModule.updateEnsure2Map = function(newMapping) {{
+ requireModule.updateEnsure2Map = function(newMapping) {{
map = newMapping;
}}
- requireModule.ensure2 = function(chunkId){{
+ requireModule.ensure2 = async function(chunkId) {{
let toEnsure = map[chunkId];
- if (toEnsure) {{
- return Promise.all(toEnsure.map(function(c){{ return requireModule.ensure(c); }}))
- }}else{{
- return Promise.resolve();
- }}
+ if (!toEnsure) return;
+ return Promise.all(toEnsure.map(c => requireModule.ensure(c)));
}};
}})();
"#📝 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.
| r#" | |
| (function(){{ | |
| let map = {ensure_map}; | |
| requireModule.updateEnsure2Map = function(newMapping) {{ | |
| map = newMapping; | |
| }} | |
| requireModule.ensure2 = function(chunkId){{ | |
| let toEnsure = map[chunkId]; | |
| if (toEnsure) {{ | |
| return Promise.all(toEnsure.map(function(c){{ return requireModule.ensure(c); }})) | |
| }}else{{ | |
| return Promise.resolve(); | |
| }} | |
| }}; | |
| }})(); | |
| "# | |
| r#" | |
| 'use strict'; | |
| (function(){{ | |
| let map = {ensure_map}; | |
| requireModule.updateEnsure2Map = function(newMapping) {{ | |
| map = newMapping; | |
| }} | |
| requireModule.ensure2 = async function(chunkId) {{ | |
| let toEnsure = map[chunkId]; | |
| if (!toEnsure) return; | |
| return Promise.all(toEnsure.map(c => requireModule.ensure(c))); | |
| }}; | |
| }})(); | |
| "# |
stormslowly
changed the title
simplify dynamic imports' ensure expression
import("./lazy.ts")will be changed fromto
the ensure expression depends on the source only,
and the mapping is recording in the
ensure2runtime api which depends only on code splitting result.the main benefits
the cost is
all the mappings move to entry chunk, entry chunk's size will increase.
so this feature may enable in dev first.
Summary by CodeRabbit
新特性
CentralChunkEnsure，增强了编译器的插件系统。central_ensure和inline_ensure方法，以支持动态导入的处理。"centralEnsure": true属性，扩展实验性功能选项。ChunkGraph结构中新增公共方法get_async_chunk_for_module，允许检索与给定模块关联的异步块。Plugin接口中新增hmr_runtime_updates方法，支持热模块替换（HMR）运行时更新。改进
ResolvedReplaceInfo类型，优化了依赖解析和替换的信息结构。generate_hmr_chunk方法，以集成模块确保功能，提升运行时代码的生成。