-
Notifications
You must be signed in to change notification settings - Fork 74
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
fix(SSU): 🐛 in case external not avaible to all entries #1698
Conversation
Walkthrough本次变更涉及对 Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
WalkthroughThis pull request addresses an issue in an internal H5 project with multiple entries, where different entries have different HTML script tags, causing some externals to be unavailable on every page. The solution involves wrapping the require statements in a Changes
|
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 (3)
crates/mako/src/plugins/central_ensure.rs (2)
64-64
: 修复了 JavaScript 代码生成中的语法错误添加了缺失的右花括号,确保了
updateEnsure2Map
函数定义的完整性。这个修复很重要,因为它:
- 修正了 JavaScript 运行时代码的语法
- 确保了动态模块加载时的正确性
- 与 PR 的目标(处理外部资源不可用的情况)相符
建议考虑以下改进:
- 为生成的 JavaScript 代码添加错误处理机制
- 在更新映射之前验证
newMapping
的格式建议添加以下错误处理逻辑:
requireModule.updateEnsure2Map = function(newMapping) {{ + if (typeof newMapping !== 'object') { + console.warn('Invalid mapping format provided'); + return; + } map = newMapping; }};
Line range hint
41-77
: 建议增强错误处理的可读性当前的错误处理实现基本完善,但建议增加更详细的错误信息,以便于调试和问题定位。
建议按照以下方式改进错误处理:
- let chunk_async_map = module_ensure_map(context)?; + let chunk_async_map = module_ensure_map(context) + .map_err(|e| anyhow!("Failed to generate module ensure map: {}", e))?; - let ensure_map = serde_json::to_string(&chunk_async_map)?; + let ensure_map = serde_json::to_string(&chunk_async_map) + .map_err(|e| anyhow!("Failed to serialize chunk async map: {}", e))?;crates/mako/src/plugins/ssu.rs (1)
Line range hint
571-603
: 修复遍历css_patch
时使用错误的变量在遍历
css_patch
时,循环中错误地使用了js_patch
作为迭代对象,导致无法正确处理 CSS 补丁映射。应将第二个循环中的迭代对象更正为css_patch
。请应用以下差异修复:
for(var key in js_patch) { chunksIdToUrlMap[key] = js_patch[key]; } -for(var key in js_patch) { +for(var key in css_patch) { cssChunksIdToUrlMap[key] = css_patch[key]; }
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1698 +/- ##
=======================================
Coverage 55.28% 55.28%
=======================================
Files 175 175
Lines 17696 17696
=======================================
Hits 9783 9783
Misses 7913 7913 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
problem
found in internal h5 project which has multi entries, different entry has different html script tag
so the external maybe not available every page
solution
wrap those require in
try catch
Summary by CodeRabbit
新特性
性能优化