Merge qiannan 8 #1553
Merge qiannan 8 #1553
Conversation
Walkthrough此次更改涉及多个文件，主要增强了应用程序的图表数据处理和WebSocket集成。新增了处理图表数据的函数，并在 Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant WebSocket
Client->>Server: 请求数据
Server->>WebSocket: 建立连接
WebSocket-->>Server: 连接成功
Server->>WebSocket: 发送统计数据
WebSocket-->>Client: 接收统计数据
Client->>Server: 更新图表数据
Server-->>Client: 返回处理结果
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 (
|
| @@ -226,7 +226,9 @@ pub struct StatsConfig { | |||
| } | |||
|
|
|||
| #[derive(Deserialize, Serialize, Clone, Debug)] | |||
| pub struct AnalyzeConfig {} | |||
| pub struct AnalyzeConfig { | |||
| pub watch: Option<bool>, | |||
There was a problem hiding this comment.
这个配置是不是可以去掉，跟随 mako 启动是否开启 watch
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1553 +/- ##
==========================================
- Coverage 61.63% 61.58% -0.06%
==========================================
Files 127 127
Lines 15294 15308 +14
==========================================
Hits 9427 9427
- Misses 5867 5881 +14 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- client/index.tsx (4 hunks)
- crates/mako/src/config/config.rs (1 hunks)
- crates/mako/src/dev.rs (8 hunks)
- crates/mako/src/generate.rs (1 hunks)
- crates/mako/src/generate/analyze.rs (2 hunks)
- crates/mako/src/visitors/env_replacer.rs (1 hunks)
Files skipped from review due to trivial changes (1)
- crates/mako/src/visitors/env_replacer.rs
Additional context used
GitHub Check: codecov/patch
crates/mako/src/generate/analyze.rs
[warning] 11-11: crates/mako/src/generate/analyze.rs#L11
Added line #L11 was not covered by testscrates/mako/src/dev.rs
[warning] 271-272: crates/mako/src/dev.rs#L271-L272
Added lines #L271 - L272 were not covered by tests
[warning] 274-274: crates/mako/src/dev.rs#L274
Added line #L274 was not covered by tests
[warning] 294-294: crates/mako/src/dev.rs#L294
Added line #L294 was not covered by tests
[warning] 467-470: crates/mako/src/dev.rs#L467-L470
Added lines #L467 - L470 were not covered by tests
[warning] 475-475: crates/mako/src/dev.rs#L475
Added line #L475 was not covered by tests
[warning] 486-486: crates/mako/src/dev.rs#L486
Added line #L486 was not covered by testscrates/mako/src/generate.rs
[warning] 196-199: crates/mako/src/generate.rs#L196-L199
Added lines #L196 - L199 were not covered by testscrates/mako/src/config/config.rs
[warning] 230-230: crates/mako/src/config/config.rs#L230
Added line #L230 was not covered by tests
Additional comments not posted (2)
crates/mako/src/generate/analyze.rs (1)
Line range hint
11-32: 增强错误处理并验证新参数的使用此函数现在包括一个新的参数
is_watch，用于控制是否启用热重载功能。建议增加错误处理，避免潜在的运行时错误。同时，需要验证整个代码库中此函数的使用情况，确保新参数被正确使用。建议对
serde_json::to_string_pretty和fs::write的调用添加错误处理逻辑，以避免程序在运行时崩溃。例如：let stats_json = serde_json::to_string_pretty(&stats).map_err(|e| e.to_string())?; let report_path = path.join("analyze-report.html"); fs::write(&report_path, html_str).map_err(|e| e.to_string())?;运行以下脚本以验证函数的使用情况：
Verification successful
验证
write_analyze函数的使用情况在代码库中，
write_analyze函数的新参数is_watch已正确使用。请确保在serde_json::to_string_pretty和fs::write的调用中添加错误处理逻辑，以避免潜在的运行时错误。
- 在
crates/mako/src/generate.rs中，write_analyze函数已使用新签名。- 建议在
serde_json::to_string_pretty和fs::write中添加错误处理。请根据建议更新代码以增强错误处理。
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：验证整个代码库中 `write_analyze` 函数的使用情况。 # 测试：搜索函数的使用。预期：只有新签名的出现。 rg --type rust -A 5 $'write_analyze'Length of output: 980
crates/mako/src/config/config.rs (1)
229-231: 审查AnalyzeConfig结构体中watch字段的添加添加的
watch字段允许配置分析过程中的特定行为，这是一个正向的改进。建议更新相关文档和示例，以展示如何使用这一新的配置选项。Tools
GitHub Check: codecov/patch
[warning] 230-230: crates/mako/src/config/config.rs#L230
Added line #L230 was not covered by tests
| const getFoamTreeData = (chartData: any) => { | ||
| const formatData = format(chartData?.chunkModules || []); | ||
| const resData = filterModulesForSize(formatData, 'statSize'); | ||
| return { groups: resData }; | ||
| }; |
There was a problem hiding this comment.
审查新添加的数据处理函数
新增的 getFoamTreeData 函数用于处理图表数据。需要确保此函数的逻辑正确无误，并且性能合理。
建议对此函数进行单元测试，以验证其正确性和性能。此外，考虑使用类型注解来增强代码的可读性和可维护性。
| // 如果开启了热更新,那么启动 websocket 服务。 | ||
| if (window?.hmrWatch) { | ||
| const socket = new WebSocket('ws://localhost:3000/__/sendStatsData'); | ||
|
|
||
| socket.addEventListener('message', (rawMessage) => { | ||
| const msg = JSON.parse(rawMessage.data); | ||
| console.log('msg==', msg); | ||
| setChartData(msg); | ||
| }); | ||
| } |
There was a problem hiding this comment.
增强 WebSocket 逻辑的健壮性
此段代码添加了 WebSocket 逻辑，用于实时更新图表数据。建议添加错误处理和连接管理逻辑，以提高代码的健壮性。
建议在 WebSocket 连接和消息处理中添加异常处理逻辑，例如：
socket.addEventListener('message', (rawMessage) => {
try {
const msg = JSON.parse(rawMessage.data);
console.log('msg==', msg);
setChartData(msg);
} catch (error) {
console.error('Failed to parse message data:', error);
}
});| "/__/sendStatsData" => { | ||
| if hyper_tungstenite::is_upgrade_request(&req) { | ||
| debug!("new websocket connection"); | ||
| let (response, websocket) = hyper_tungstenite::upgrade(req, None).unwrap(); | ||
| let txws = txws.clone(); | ||
| tokio_runtime::spawn(async move { | ||
| let receiver = txws.subscribe(); | ||
| if txws.receiver_count() > 0 { | ||
| let stats = compiler.create_stats_info(); | ||
| let json_data = serde_json::to_string_pretty(&stats).unwrap(); | ||
| txws.send(WsMessage { | ||
| hash: 0, | ||
| stats_data: json_data, | ||
| }) | ||
| .unwrap(); | ||
| } | ||
| Self::handle_websocket_stats_data(websocket, receiver) | ||
| .await | ||
| .unwrap(); | ||
| }); | ||
| Ok(response) | ||
| } else { |
There was a problem hiding this comment.
审查新的 WebSocket 端点和消息处理逻辑
此代码段添加了一个新的 WebSocket 端点 /__/sendStatsData，用于处理实时统计数据的发送。建议增强安全性和错误处理。
建议在 WebSocket 消息处理中添加更严格的错误处理和安全检查，例如验证消息的完整性和格式。此外，考虑对敏感数据进行加密处理，以保护在 WebSocket 通信中传输的数据。
| // 发送热更新之后的数据 | ||
| async fn handle_websocket_stats_data( | ||
| websocket: hyper_tungstenite::HyperWebsocket, | ||
| mut receiver: broadcast::Receiver<WsMessage>, | ||
| ) -> Result<()> { | ||
| let websocket = websocket.await?; | ||
| let (mut sender, mut ws_recv) = websocket.split(); | ||
| let task = tokio_runtime::spawn(async move { | ||
| loop { | ||
| if let Ok(msg) = receiver.recv().await { | ||
| if sender.send(Message::text(msg.stats_data)).await.is_err() { | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| while let Some(message) = ws_recv.next().await { | ||
| if let Ok(Message::Close(_)) = message { | ||
| break; | ||
| } | ||
| } | ||
| debug!("websocket connection disconnected"); | ||
| task.abort(); | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
增强 WebSocket 统计数据处理功能的健壮性
此函数 handle_websocket_stats_data 负责处理通过 WebSocket 发送的统计数据。建议增加错误处理和资源管理逻辑，以提高代码的健壮性和性能。
建议在此函数中添加对 WebSocket 连接和消息发送的异常处理逻辑，以防止潜在的资源泄露和运行时错误。例如：
if let Ok(msg) = receiver.recv().await {
if sender.send(Message::text(msg.stats_data)).await.is_err() {
break;
}
} else {
// 处理接收错误
debug!("Failed to receive message");
}Tools
GitHub Check: codecov/patch
[warning] 271-272: crates/mako/src/dev.rs#L271-L272
Added lines #L271 - L272 were not covered by tests
[warning] 274-274: crates/mako/src/dev.rs#L274
Added line #L274 was not covered by tests
[warning] 294-294: crates/mako/src/dev.rs#L294
Added line #L294 was not covered by tests
| let analyze = &self.context.config.analyze.clone().unwrap(); | ||
| let mut is_watch = false; | ||
| if analyze.watch.is_some() && analyze.watch.unwrap() { | ||
| is_watch = true; | ||
| } | ||
| Analyze::write_analyze(&stats, &self.context.config.output.path, is_watch)?; |
There was a problem hiding this comment.
审查 write_analyze 方法调用的更改
代码正确地添加了 is_watch 参数来根据配置处理不同的行为。建议显式处理 analyze.watch 为 None 的情况，虽然当前逻辑默认为 false。此外，由于这是一个新逻辑，建议增加相关的单元测试以确保功能的正确性和鲁棒性。
- if analyze.watch.is_some() && analyze.watch.unwrap() {
+ if analyze.watch.unwrap_or(false) {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.
| let analyze = &self.context.config.analyze.clone().unwrap(); | |
| let mut is_watch = false; | |
| if analyze.watch.is_some() && analyze.watch.unwrap() { | |
| is_watch = true; | |
| } | |
| Analyze::write_analyze(&stats, &self.context.config.output.path, is_watch)?; | |
| let analyze = &self.context.config.analyze.clone().unwrap(); | |
| let mut is_watch = false; | |
| if analyze.watch.unwrap_or(false) { | |
| is_watch = true; | |
| } | |
| Analyze::write_analyze(&stats, &self.context.config.output.path, is_watch)?; |
Tools
GitHub Check: codecov/patch
[warning] 196-199: crates/mako/src/generate.rs#L196-L199
Added lines #L196 - L199 were not covered by tests
Summary by CodeRabbit
新功能
修复