fix: hmr with magic comment chunk name #1663
Conversation
Walkthrough本次更改主要涉及对多个文件的修改，以改善块类型的处理和JavaScript块的生成。在 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
e2e/fixtures/magic-comments.chunk-name/src/lazy_a_0.ts (1)
4-4: 建议添加错误处理逻辑动态导入可能会失败，建议添加错误处理来提高代码的健壮性。
建议修改为：
-import('./lazy_inner'); +import('./lazy_inner').catch(error => { + console.error('Failed to load lazy_inner:', error); +});e2e/fixtures/magic-comments.chunk-name/src/lazy_b.ts (1)
4-4: 动态导入未处理返回的 Promise动态导入会返回一个 Promise，当前代码没有处理这个 Promise。虽然这可能是有意为之（因为这是测试文件），但建议添加注释说明这是预期行为。
建议添加注释说明：
+// 注意：此处故意不等待动态导入的结果，用于测试 chunk 加载机制 import('./lazy_inner');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
- crates/mako/src/generate/chunk_pot/str_impl.rs (2 hunks)
- crates/mako/src/generate/group_chunk.rs (2 hunks)
- e2e/fixtures/magic-comments.chunk-name/expect.js (1 hunks)
- e2e/fixtures/magic-comments.chunk-name/public/index.html (1 hunks)
- e2e/fixtures/magic-comments.chunk-name/src/lazy_a_0.ts (1 hunks)
- e2e/fixtures/magic-comments.chunk-name/src/lazy_b.ts (1 hunks)
- e2e/fixtures/magic-comments.chunk-name/src/lazy_inner.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- e2e/fixtures/magic-comments.chunk-name/public/index.html
- e2e/fixtures/magic-comments.chunk-name/src/lazy_inner.ts
🔇 Additional comments (5)
e2e/fixtures/magic-comments.chunk-name/expect.js (2)
7-8: 测试用例补充合理，验证了chunk_a-async.js的动态导入功能测试用例通过验证
console.log和__mako_require__.ensure的存在，确保了魔法注释命名的chunk中的动态导入功能正常工作。
12-13: 测试用例完善，验证了chunk_b-async.js的动态导入实现测试断言确保了chunk中包含预期的日志输出和动态导入代码，有效验证了PR修复的问题。
crates/mako/src/generate/chunk_pot/str_impl.rs (2)
14-14: 导入更改看起来很合理！新增的导入项
ChunkType和generate_module_id与代码中的使用场景相符，支持了新的分块类型处理功能。Also applies to: 19-19
71-77: 验证HMR功能在不同类型的块中的行为使用
chunk_root_module_id初始化cssInstalledChunks的改动符合预期，但建议验证这个改动在所有场景下的行为。✅ Verification successful
使用
chunk_root_module_id初始化cssInstalledChunks的改动已验证安全代码分析显示：
cssInstalledChunks在运行时模板中被正确使用，用于CSS块的加载和HMR功能- 新的
chunk_root_module_id实现考虑了不同类型的块（Entry和Worker），确保了正确的模块ID生成- 没有发现任何地方仍在使用旧的
pot.chunk_id来初始化cssInstalledChunks🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：验证不同类型块的HMR功能 # 测试：搜索所有使用cssInstalledChunks的地方 rg -l 'cssInstalledChunks' # 测试：检查是否有其他地方仍在使用旧的chunk_id rg -l 'pot\.chunk_id.*cssInstalledChunks'Length of output: 203
Script:
#!/bin/bash # 检查cssInstalledChunks的具体使用上下文 rg -C 5 'cssInstalledChunks' crates/mako/templates/app_runtime.stpl rg -C 5 'cssInstalledChunks' crates/mako/src/generate/chunk_pot/ast_impl.rs # 检查chunk_root_module_id的使用情况 rg -C 3 'chunk_root_module_id' crates/mako/src/generate/chunk_pot/str_impl.rs rg -C 3 'chunk_root_module_id' crates/mako/src/generate/chunk_pot/ast_impl.rsLength of output: 3288
crates/mako/src/generate/group_chunk.rs (1)
119-140: 依赖边处理逻辑优化这个改动通过将边的扩展逻辑移到chunk存在性检查之前，确保了所有依赖关系都能被正确追踪。建议考虑以下几点改进：
- 在边扩展过程中添加错误处理，以防止潜在的无效chunk名称。
- 考虑添加日志记录，以便于调试和监控依赖关系的变化。
✅ Verification successful
依赖处理逻辑已得到适当优化
根据代码分析，当前的依赖边处理逻辑已经实现了以下关键特性：
- 通过
visited_workers集合有效防止了循环依赖问题- 在
handle_dynamic_dependencies和handle_worker_dependencies中实现了完整的依赖去重- 边的扩展逻辑位于 chunk 存在性检查之前是合理的，确保了依赖关系的完整性
建议的错误处理和日志记录优化并不是必需的，因为：
- chunk 名称已经在
get_chunk_name()中得到妥善处理- 现有的依赖处理机制已经能够很好地处理各种边界情况
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 验证是否存在潜在的循环依赖 rg -U "edges.extend.*chunk.id" -A 10 -B 10Length of output: 44
Script:
#!/bin/bash # 搜索edges相关的代码实现 rg -U "edges\.extend" -A 10 -B 10 # 搜索chunk_graph相关的实现 ast-grep --pattern 'impl$_ ChunkGraph { $$$ }' # 查找依赖处理相关的代码 rg -U "dependencies" -A 5 -B 5 crates/mako/src/generate/Length of output: 27482
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1663 +/- ##
==========================================
- Coverage 55.61% 55.59% -0.02%
==========================================
Files 173 173
Lines 17502 17508 +6
==========================================
Hits 9733 9733
- Misses 7769 7775 +6 ☔ View full report in Codecov by Sentry. |
If there is a dynamic import in chunk which named with a magic comment, the runtime will be broken and hmr will fail.
Summary by CodeRabbit
新特性
lazy_inner模块的功能。错误修复
文档
index.html文件，包含基本的 HTML 结构和脚本加载。样式