Skip to content
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

Merge qiannan 8 #1553

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion client/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ function App() {
// ref 用于保存Treemap实例
const treeMapRef = useRef(null);
const [chartData, setChartData] = useState('');

// toolTip展示使用
const [tooltipContent, setToolTipContent] = useState('');
const createModulesTree = (modules) => {
Expand Down Expand Up @@ -106,9 +105,15 @@ function App() {
</div>
);
};
const getFoamTreeData = (chartData: any) => {
const formatData = format(chartData?.chunkModules || []);
const resData = filterModulesForSize(formatData, 'statSize');
return { groups: resData };
};
Comment on lines +108 to +112
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查新添加的数据处理函数

新增的 getFoamTreeData 函数用于处理图表数据。需要确保此函数的逻辑正确无误,并且性能合理。

建议对此函数进行单元测试,以验证其正确性和性能。此外,考虑使用类型注解来增强代码的可读性和可维护性。

const createFoamTree = (chartData: any) => {
const formatData = format(chartData?.chunkModules || []);
const resData = filterModulesForSize(formatData, 'statSize');
debugger;
return new FoamTree({
element: chartRef.current,
layout: 'squarified',
Expand Down Expand Up @@ -161,6 +166,16 @@ function App() {
useEffect(() => {
window.addEventListener('load', () => {
setChartData(window?.chartData);
// 如果开启了热更新,那么启动 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);
});
}
Comment on lines +169 to +178
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增强 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);
  }
});

});
window.addEventListener('resize', resize);
return () => {
Expand All @@ -172,7 +187,22 @@ function App() {
console.warn('数据未初始化!!');
return;
}
// 如果已经实例化并且 chartData 发生改变,那么就重新设置值。
if (treeMapRef.current) {
debugger;
treeMapRef.current.set({
dataObject: getFoamTreeData(chartData),
});
treeMapRef.current.update();
return;
}
treeMapRef.current = createFoamTree(chartData);
return () => {
if (treeMapRef.current) {
treeMapRef.current.dispose();
treeMapRef.current = null;
}
};
}, [chartData]);
return (
<>
Expand Down
4 changes: 3 additions & 1 deletion crates/mako/src/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@
}

#[derive(Deserialize, Serialize, Clone, Debug)]
pub struct AnalyzeConfig {}
pub struct AnalyzeConfig {
pub watch: Option<bool>,

Check warning on line 230 in crates/mako/src/config/config.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/config/config.rs#L230

Added line #L230 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个配置是不是可以去掉,跟随 mako 启动是否开启 watch

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个具体怎么整啊

}

#[derive(Deserialize, Serialize, Clone, Debug)]
pub enum CodeSplittingStrategy {
Expand Down
73 changes: 69 additions & 4 deletions crates/mako/src/dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
let root = self.root.clone();
let compiler = self.compiler.clone();
let txws_watch = txws.clone();

if self.compiler.context.config.dev_server.is_some() {
std::thread::spawn(move || {
if let Err(e) = Self::watch_for_changes(root, compiler, txws_watch) {
Expand All @@ -50,7 +49,7 @@
} else if let Err(e) = Self::watch_for_changes(root, compiler, txws_watch) {
eprintln!("Error watching files: {:?}", e);
}

let compiler = self.compiler.clone();
// server
if self.compiler.context.config.dev_server.is_some() {
let config_port = self
Expand All @@ -67,14 +66,19 @@
let txws = txws.clone();
let make_svc = make_service_fn(move |_conn| {
let context = context.clone();
let compiler = compiler.clone();
let txws = txws.clone();
async move {
Ok::<_, hyper::Error>(service_fn(move |req| {
let context = context.clone();
let txws = txws.clone();
let compile = compiler.clone();

let staticfile =
hyper_staticfile::Static::new(context.config.output.path.clone());
async move { Self::handle_requests(req, context, staticfile, txws).await }
async move {
Self::handle_requests(req, context, staticfile, txws, compile).await
}
}))
}
});
Expand Down Expand Up @@ -120,6 +124,7 @@
context: Arc<Context>,
staticfile: hyper_staticfile::Static,
txws: broadcast::Sender<WsMessage>,
compiler: Arc<Compiler>,
) -> Result<hyper::Response<Body>> {
debug!("> {} {}", req.method().to_string(), req.uri().path());

Expand Down Expand Up @@ -158,6 +163,31 @@
Ok(not_found_response())
}
}
"/__/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 {
Comment on lines +166 to +187
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查新的 WebSocket 端点和消息处理逻辑

此代码段添加了一个新的 WebSocket 端点 /__/sendStatsData,用于处理实时统计数据的发送。建议增强安全性和错误处理。

建议在 WebSocket 消息处理中添加更严格的错误处理和安全检查,例如验证消息的完整性和格式。此外,考虑对敏感数据进行加密处理,以保护在 WebSocket 通信中传输的数据。

Ok(not_found_response())
}
}
_ => {
// for bundle outputs

Expand Down Expand Up @@ -237,6 +267,32 @@
}
}

// 发送热更新之后的数据
async fn handle_websocket_stats_data(
websocket: hyper_tungstenite::HyperWebsocket,

Check warning on line 272 in crates/mako/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/dev.rs#L271-L272

Added lines #L271 - L272 were not covered by tests
mut receiver: broadcast::Receiver<WsMessage>,
) -> Result<()> {

Check warning on line 274 in crates/mako/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/dev.rs#L274

Added line #L274 was not covered by tests
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(())
}

Check warning on line 294 in crates/mako/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/dev.rs#L294

Added line #L294 was not covered by tests
Comment on lines +270 to +294
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

增强 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


// TODO: refact socket message data structure
async fn handle_websocket(
websocket: hyper_tungstenite::HyperWebsocket,
Expand Down Expand Up @@ -407,8 +463,16 @@

let receiver_count = txws.receiver_count();
debug!("receiver count: {}", receiver_count);
// 解析更新完之后的数据
let stats = compiler.create_stats_info();
let json_data = serde_json::to_string_pretty(&stats).unwrap();
let ws_new_data = WsMessage {
hash: **hmr_hash,

Check warning on line 470 in crates/mako/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/dev.rs#L467-L470

Added lines #L467 - L470 were not covered by tests
stats_data: json_data,
};

if receiver_count > 0 {
txws.send(WsMessage { hash: **hmr_hash }).unwrap();
txws.send(ws_new_data).unwrap();

Check warning on line 475 in crates/mako/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/dev.rs#L475

Added line #L475 was not covered by tests
debug!("send message to clients");
}

Expand All @@ -419,4 +483,5 @@
#[derive(Clone, Debug)]
struct WsMessage {
hash: u64,
stats_data: String,

Check warning on line 486 in crates/mako/src/dev.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/dev.rs#L486

Added line #L486 was not covered by tests
}
7 changes: 6 additions & 1 deletion crates/mako/src/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,12 @@
}

if self.context.config.analyze.is_some() {
Analyze::write_analyze(&stats, &self.context.config.output.path)?;
let analyze = &self.context.config.analyze.clone().unwrap();
let mut is_watch = false;
if analyze.watch.is_some() && analyze.watch.unwrap() {
is_watch = true;

Check warning on line 199 in crates/mako/src/generate.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/generate.rs#L196-L199

Added lines #L196 - L199 were not covered by tests
}
Analyze::write_analyze(&stats, &self.context.config.output.path, is_watch)?;
Comment on lines +196 to +201
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查 write_analyze 方法调用的更改

代码正确地添加了 is_watch 参数来根据配置处理不同的行为。建议显式处理 analyze.watchNone 的情况,虽然当前逻辑默认为 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.

Suggested change
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

}

debug!("generate done in {}ms", t_generate.elapsed().as_millis());
Expand Down
4 changes: 3 additions & 1 deletion crates/mako/src/generate/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
pub struct Analyze {}

impl Analyze {
pub fn write_analyze(stats: &StatsJsonMap, path: &Path) -> Result<()> {
pub fn write_analyze(stats: &StatsJsonMap, path: &Path, is_watch: bool) -> Result<()> {

Check warning on line 11 in crates/mako/src/generate/analyze.rs

View check run for this annotation

Codecov / codecov/patch

crates/mako/src/generate/analyze.rs#L11

Added line #L11 was not covered by tests
let stats_json = serde_json::to_string_pretty(&stats).unwrap();
let html_str = format!(
r#"<!DOCTYPE html>
Expand All @@ -22,12 +22,14 @@
<div id="root"></div>
<script>
window.chartData = {};
window.hmrWatch = {}
</script>
<script>{}</script>
</body>
</html>"#,
include_str!("../../../../client/dist/index.css"),
stats_json,
is_watch,
include_str!("../../../../client/dist/index.js").replace("</script>", "<\\/script>")
);
let report_path = path.join("analyze-report.html");
Expand Down
2 changes: 1 addition & 1 deletion crates/mako/src/visitors/env_replacer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ mod tests {
}

#[test]
fn test_complited_computed_as_member_key() {
fn test_completed_computed_as_member_key() {
assert_eq!(
run(
r#"log(A[v.v])"#,
Expand Down
Loading