feat(minifier): implement block stmt support for StatementFusion (#…

…6422)
oxc-project · Oct 10, 2024 · 9dc4ee9 · 9dc4ee9
1 parent f70e93b
commit 9dc4ee9
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 38 deletions.
diff --git a/crates/oxc_minifier/src/ast_passes/statement_fusion.rs b/crates/oxc_minifier/src/ast_passes/statement_fusion.rs
@@ -68,20 +68,20 @@ impl<'a> StatementFusion {
             | Statement::ThrowStatement(_)
             | Statement::SwitchStatement(_) => true,
             Statement::ReturnStatement(return_stmt) => return_stmt.argument.is_some(),
-            // Statement::ForStatement(for_stmt) => {
-            // // Avoid cases where we have for(var x;_;_) { ....
-            // for_stmt.init.is_none()
-            // || for_stmt.init.as_ref().is_some_and(ForStatementInit::is_expression)
-            // }
-            // Statement::ForInStatement(for_in_stmt) => {
-            // TODO
-            // }
+            Statement::ForStatement(for_stmt) => {
+                // Avoid cases where we have for(var x;_;_) { ....
+                for_stmt.init.is_none()
+                    || for_stmt.init.as_ref().is_some_and(ForStatementInit::is_expression)
+            }
+            // TODO: support for-in, we need to check the init for side effects
+            Statement::ForInStatement(_for_in_stmt) => false,
             Statement::LabeledStatement(labeled_stmt) => {
                 Self::is_fusable_control_statement(&labeled_stmt.body)
             }
-            // Statement::BlockStatement(_) => {
-            // TODO
-            // }
+            Statement::BlockStatement(block) => {
+                can_merge_block_stmt(block)
+                    && block.body.first().map_or(false, Self::is_fusable_control_statement)
+            }
             _ => false,
         }
     }
@@ -133,17 +133,17 @@ impl<'a> StatementFusion {
             Statement::ThrowStatement(throw_stmt) => &mut throw_stmt.argument,
             Statement::SwitchStatement(switch_stmt) => &mut switch_stmt.discriminant,
             Statement::ReturnStatement(return_stmt) => return_stmt.argument.as_mut().unwrap(),
-            // Statement::ForStatement(for_stmt) => {
-            // if let Some(init) = for_stmt.init.as_mut() {
-            // init.as_expression_mut().unwrap()
-            // } else {
-            // for_stmt.init =
-            // Some(ctx.ast.for_statement_init_expression(
-            // ctx.ast.expression_sequence(SPAN, exprs),
-            // ));
-            // return;
-            // }
-            // }
+            Statement::ForStatement(for_stmt) => {
+                if let Some(init) = for_stmt.init.as_mut() {
+                    init.as_expression_mut().unwrap()
+                } else {
+                    for_stmt.init =
+                        Some(ctx.ast.for_statement_init_expression(
+                            ctx.ast.expression_sequence(SPAN, exprs),
+                        ));
+                    return;
+                }
+            }
             Statement::LabeledStatement(labeled_stmt) => {
                 Self::fuse_expression_into_control_flow_statement(
                     &mut labeled_stmt.body,
@@ -152,6 +152,14 @@ impl<'a> StatementFusion {
                 );
                 return;
             }
+            Statement::BlockStatement(block) => {
+                Self::fuse_expression_into_control_flow_statement(
+                    block.body.first_mut().unwrap(),
+                    exprs,
+                    ctx,
+                );
+                return;
+            }
             _ => {
                 unreachable!("must match with `Self::is_fusable_control_statement`");
             }
@@ -161,6 +169,21 @@ impl<'a> StatementFusion {
     }
 }
 
+fn can_merge_block_stmt(node: &BlockStatement) -> bool {
+    return node.body.iter().all(can_merge_block_stmt_member);
+}
+
+fn can_merge_block_stmt_member(node: &Statement) -> bool {
+    match node {
+        Statement::LabeledStatement(label) => can_merge_block_stmt_member(&label.body),
+        Statement::VariableDeclaration(var_decl) => {
+            !matches!(var_decl.kind, VariableDeclarationKind::Const | VariableDeclarationKind::Let)
+        }
+        Statement::ClassDeclaration(_) | Statement::FunctionDeclaration(_) => false,
+        _ => true,
+    }
+}
+
 #[cfg(test)]
 mod test {
     use oxc_allocator::Allocator;
@@ -252,7 +275,6 @@ mod test {
     }
 
     #[test]
-    #[ignore]
     fn fuse_into_vanilla_for1() {
         fuse("a;b;c;for(;g;){}", "for(a,b,c;g;){}");
         fuse("a;b;c;for(d;g;){}", "for(a,b,c,d;g;){}");
@@ -261,7 +283,6 @@ mod test {
     }
 
     #[test]
-    #[ignore]
     fn fuse_into_vanilla_for2() {
         fuse_same("a;b;c;for(var d;g;){}");
         fuse_same("a;b;c;for(let d;g;){}");
@@ -277,7 +298,6 @@ mod test {
     }
 
     #[test]
-    #[ignore]
     fn fuse_into_block() {
         fuse("a;b;c;{d;e;f}", "{a,b,c,d,e,f}");
         fuse(
@@ -299,7 +319,6 @@ mod test {
     }
 
     #[test]
-    #[ignore]
     fn no_fuse_into_block() {
         // Never fuse a statement into a block that contains let/const/class declarations, or you risk
         // colliding variable names. (unless the AST is normalized).
@@ -312,14 +331,14 @@ mod test {
         fuse_same("a; { b; const otherVariable = 1; }");
 
         // enable_normalize();
-        test(
-            "function f(a) { if (COND) { a; { b; let a = 1; } } }",
-            "function f(a) { if (COND) { { a,b; let a$jscomp$1 = 1; } } }",
-        );
-        test(
-            "function f(a) { if (COND) { a; { b; let otherVariable = 1; } } }",
-            "function f(a) { if (COND) {  { a,b; let otherVariable = 1; } } }",
-        );
+        // test(
+        //     "function f(a) { if (COND) { a; { b; let a = 1; } } }",
+        //     "function f(a) { if (COND) { { a,b; let a$jscomp$1 = 1; } } }",
+        // );
+        // test(
+        //     "function f(a) { if (COND) { a; { b; let otherVariable = 1; } } }",
+        //     "function f(a) { if (COND) {  { a,b; let otherVariable = 1; } } }",
+        // );
     }
 
     #[test]

diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap
@@ -2,23 +2,23 @@ Original   | Minified   | esbuild    | Gzip       | esbuild
 
 72.14 kB   | 24.46 kB   | 23.70 kB   | 8.65 kB    | 8.54 kB    | react.development.js
 
-173.90 kB  | 61.68 kB   | 59.82 kB   | 19.54 kB   | 19.33 kB   | moment.js 
+173.90 kB  | 61.68 kB   | 59.82 kB   | 19.53 kB   | 19.33 kB   | moment.js 
 
 287.63 kB  | 92.83 kB   | 90.07 kB   | 32.29 kB   | 31.95 kB   | jquery.js 
 
 342.15 kB  | 124.11 kB  | 118.14 kB  | 44.80 kB   | 44.37 kB   | vue.js    
 
 544.10 kB  | 74.13 kB   | 72.48 kB   | 26.23 kB   | 26.20 kB   | lodash.js 
 
-555.77 kB  | 278.23 kB  | 270.13 kB  | 91.36 kB   | 90.80 kB   | d3.js     
+555.77 kB  | 278.22 kB  | 270.13 kB  | 91.36 kB   | 90.80 kB   | d3.js     
 
 1.01 MB    | 470.11 kB  | 458.89 kB  | 126.97 kB  | 126.71 kB  | bundle.min.js
 
 1.25 MB    | 670.96 kB  | 646.76 kB  | 164.72 kB  | 163.73 kB  | three.js  
 
-2.14 MB    | 756.33 kB  | 724.14 kB  | 182.74 kB  | 181.07 kB  | victory.js
+2.14 MB    | 756.32 kB  | 724.14 kB  | 182.74 kB  | 181.07 kB  | victory.js
 
-3.20 MB    | 1.05 MB    | 1.01 MB    | 334.07 kB  | 331.56 kB  | echarts.js
+3.20 MB    | 1.05 MB    | 1.01 MB    | 334.08 kB  | 331.56 kB  | echarts.js
 
 6.69 MB    | 2.44 MB    | 2.31 MB    | 498.88 kB  | 488.28 kB  | antd.js