feat(minifier): improve minimizing conditionals

oxc-project · Jan 3, 2025 · 4bd8113 · 4bd8113
1 parent c90fc16
commit 4bd8113
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 13 deletions.
diff --git a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs
@@ -275,6 +275,9 @@ impl<'a> PeepholeMinimizeConditions {
                 Expression::BooleanLiteral(consequent_lit),
             ) = (&expr.left, &expr.right)
             {
+                if !is_in_boolean_context(ctx) {
+                    return None;
+                }
                 if consequent_lit.value {
                     return Some(ctx.ast.move_expression(&mut expr.left));
                 }
@@ -354,6 +357,8 @@ impl<'a> PeepholeMinimizeConditions {
             }
         }
 
+        let in_boolean_context = is_in_boolean_context(ctx);
+
         // `a ? false : true` -> `!a`
         // `a ? true : false` -> `!!a`
         if let (
@@ -373,6 +378,10 @@ impl<'a> PeepholeMinimizeConditions {
                 }
                 (true, false) => {
                     let ident = ctx.ast.move_expression(&mut expr.test);
+
+                    if in_boolean_context {
+                        return Some(ident);
+                    }
                     return Some(ctx.ast.expression_unary(
                         expr.span,
                         UnaryOperator::LogicalNot,
@@ -383,14 +392,6 @@ impl<'a> PeepholeMinimizeConditions {
             }
         }
 
-        let in_boolean_context = matches!(
-            ctx.parent(),
-            Ancestor::IfStatementTest(_)
-                | Ancestor::WhileStatementTest(_)
-                | Ancestor::DoWhileStatementTest(_)
-                | Ancestor::ExpressionStatementExpression(_)
-        );
-
         // `x ? true : y` -> `x || y`
         // `x ? false : y` -> `!x && y`
         if let (Expression::Identifier(_), Expression::BooleanLiteral(consequent_lit), _) =
@@ -457,6 +458,38 @@ impl<'a> PeepholeMinimizeConditions {
     }
 }
 
+// returns `true` if the current node is in a context in which the return
+// value type is coerced to boolean.
+// For example `if (condition)` and `return condition`
+// inside the `if` stmt, `condition` is coerced to a boolean
+// whereas inside the return, it is not
+fn is_in_boolean_context(ctx: &mut TraverseCtx<'_>) -> bool {
+    for ancestor in ctx.ancestors() {
+        match ancestor {
+            Ancestor::IfStatementTest(_)
+            | Ancestor::WhileStatementTest(_)
+            | Ancestor::DoWhileStatementTest(_)
+            | Ancestor::ExpressionStatementExpression(_) => return true,
+            Ancestor::CallExpressionArguments(_)
+            | Ancestor::AssignmentPatternRight(_)
+            | Ancestor::BindingRestElementArgument(_)
+            | Ancestor::JSXSpreadAttributeArgument(_)
+            | Ancestor::NewExpressionArguments(_)
+            | Ancestor::ObjectPropertyKey(_)
+            | Ancestor::ObjectPropertyValue(_)
+            | Ancestor::ReturnStatementArgument(_)
+            | Ancestor::ThrowStatementArgument(_)
+            | Ancestor::YieldExpressionArgument(_)
+            | Ancestor::VariableDeclaratorInit(_) => return false,
+            _ => continue,
+        }
+    }
+    #[cfg(debug_assertions)]
+    unreachable!();
+    #[cfg(not(debug_assertions))]
+    false
+}
+
 /// <https://github.com/google/closure-compiler/blob/v20240609/test/com/google/javascript/jscomp/PeepholeMinimizeConditionsTest.java>
 #[cfg(test)]
 mod test {
@@ -754,11 +787,10 @@ mod test {
 
     #[test]
     fn test_minimize_expr_condition() {
-        fold("(x ? true : false) && y()", "!!x && y()");
+        fold("(x ? true : false) && y()", "x && y()");
         fold("(x ? false : true) && y()", "!x && y()");
-        fold("(x ? true : y) && y()", "(!!x || y) && y()");
-        // TODO: drop the `!!`
-        fold("(x ? y : false) && y()", "(!!x && y) && y()");
+        fold("(x ? true : y) && y()", "(x || y) && y()");
+        fold("(x ? y : false) && y()", "(x && y) && y()");
         fold("var x; (x && true) && y()", "var x; x && y()");
         fold("var x; (x && false) && y()", "var x; false && y()");
         fold("(x && true) && y()", "x && y()");
@@ -772,6 +804,11 @@ mod test {
         fold("let x = foo ? true : false", "let x = !!foo");
         fold("let x = foo ? true : bar", "let x = !!foo || bar");
         fold("let x = foo ? bar : false", "let x = !!foo && bar");
+        fold("function x () { return a ? true : false }", "function x() { return !!a }");
+        fold("function x () { return a ? false : true }", "function x() { return !a }");
+        fold("function x () { return a ? true : b }", "function x() { return !!a || b }");
+        // can't be minified e.g. `a = ''` would return `''`
+        fold("function x() { return a && true }", "function x() { return a && true }");
     }
 
     #[test]

diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap
@@ -21,7 +21,7 @@ Original   | minified   | minified   | gzip       | gzip       | Fixture
 
 3.20 MB    | 1.01 MB    | 1.01 MB    | 331.90 kB  | 331.56 kB  | echarts.js
 
-6.69 MB    | 2.32 MB    | 2.31 MB    | 492.82 kB  | 488.28 kB  | antd.js   
+6.69 MB    | 2.32 MB    | 2.31 MB    | 492.81 kB  | 488.28 kB  | antd.js   
 
 10.95 MB   | 3.50 MB    | 3.49 MB    | 909.30 kB  | 915.50 kB  | typescript.js