feat(minifier): always put literals on the rhs of equal op 1==x => …

…`x==1`
oxc-project · Jan 4, 2025 · 83e7f11 · 83e7f11
1 parent bf0fbce
commit 83e7f11
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 21 deletions.
diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs
@@ -72,6 +72,11 @@ impl<'a> Expression<'a> {
         matches!(self, Self::StringLiteral(_) | Self::TemplateLiteral(_))
     }
 
+    /// Return `true` if the expression is a plain template.
+    pub fn is_no_substitution_template(&self) -> bool {
+        matches!(self, Expression::TemplateLiteral(e) if e.is_no_substitution_template())
+    }
+
     /// Returns `true` for [numeric](NumericLiteral) and [big int](BigIntLiteral) literals.
     pub fn is_number_literal(&self) -> bool {
         matches!(self, Self::NumericLiteral(_) | Self::BigIntLiteral(_))

diff --git a/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs b/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs
@@ -748,10 +748,10 @@ mod test {
         test("null === undefined", "false");
         test("null === null", "true");
         test("null === void 0", "false");
-        test_same("null===x");
+        test_same("x===null");
 
-        test_same("null==this");
-        test_same("null==x");
+        test_same("this==null");
+        test_same("x==null");
 
         test("null != undefined", "false");
         test("null != null", "false");
@@ -769,8 +769,8 @@ mod test {
         test("null !== void 0", "true");
         test("null !== null", "false");
 
-        test_same("null!=this");
-        test_same("null!=x");
+        test_same("this!=null");
+        test_same("x!=null");
 
         test("null < null", "false");
         test("null > null", "false");
@@ -835,9 +835,7 @@ mod test {
         test("null != (function(){})", "true");
 
         test_same("({a:f()})==null");
-        test_same("null=={a:f()}");
         test_same("[f()]==null");
-        test_same("null==[f()]");
 
         test_same("this==null");
         test_same("x==null");
@@ -897,10 +895,7 @@ mod test {
         test("typeof function() {} < typeof function() {}", "false");
         test("'a' == 'a'", "true");
         test("'b' != 'a'", "true");
-        test_same("'undefined' == typeof a");
         test_same("typeof a != 'number'");
-        test_same("'undefined' == typeof a");
-        test_same("'undefined' == typeof a");
         test_same("typeof a == typeof a");
         test("'a' === 'a'", "true");
         test("'b' !== 'a'", "true");
@@ -1019,8 +1014,7 @@ mod test {
     #[test]
     fn test_object_bigint_comparison() {
         test_same("{ valueOf: function() { return 0n; } } != 0n");
-        test_same("0n != { valueOf: function() { return 0n; } }");
-        test_same("0n != { toString: function() { return '0'; } }");
+        test_same("{ toString: function() { return '0'; } } != 0n");
     }
 
     #[test]

diff --git a/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs b/crates/oxc_minifier/src/ast_passes/peephole_substitute_alternate_syntax.rs
@@ -84,7 +84,10 @@ impl<'a> Traverse<'a> for PeepholeSubstituteAlternateSyntax {
         match expr {
             Expression::ArrowFunctionExpression(e) => self.try_compress_arrow_expression(e, ctx),
             Expression::ChainExpression(e) => self.try_compress_chain_call_expression(e, ctx),
-            Expression::BinaryExpression(e) => self.try_compress_type_of_equal_string(e),
+            Expression::BinaryExpression(e) => {
+                Self::swap_binary_expressions(e);
+                self.try_compress_type_of_equal_string(e);
+            }
             _ => {}
         }
 
@@ -114,6 +117,15 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
         Self { in_fixed_loop, in_define_export: false, changed: false }
     }
 
+    fn swap_binary_expressions(e: &mut BinaryExpression<'a>) {
+        if e.operator.is_equality()
+            && (e.left.is_literal() || e.left.is_no_substitution_template())
+            && !e.right.is_literal()
+        {
+            std::mem::swap(&mut e.left, &mut e.right);
+        }
+    }
+
     /// Test `Object.defineProperty(exports, ...)`
     fn is_object_define_property_exports(call_expr: &CallExpression<'a>) -> bool {
         let Some(Argument::Identifier(ident)) = call_expr.arguments.first() else { return false };
@@ -623,12 +635,6 @@ impl<'a, 'b> PeepholeSubstituteAlternateSyntax {
 
     /// `typeof foo === 'number'` -> `typeof foo == 'number'`
     fn try_compress_type_of_equal_string(&mut self, e: &mut BinaryExpression<'a>) {
-        // Change  `'undefined' == typeof _'` -> `typeof _ == 'undefined'`
-        if matches!(&e.right, Expression::UnaryExpression(unary_expr) if unary_expr.operator.is_typeof())
-            && e.left.is_string_literal()
-        {
-            std::mem::swap(&mut e.left, &mut e.right);
-        }
         let op = match e.operator {
             BinaryOperator::StrictEquality => BinaryOperator::Equality,
             BinaryOperator::StrictInequality => BinaryOperator::Inequality,

diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap
@@ -19,9 +19,9 @@ Original   | minified   | minified   | gzip       | gzip       | Fixture
 
 2.14 MB    | 726.19 kB  | 724.14 kB  | 180.18 kB  | 181.07 kB  | victory.js
 
-3.20 MB    | 1.01 MB    | 1.01 MB    | 331.90 kB  | 331.56 kB  | echarts.js
+3.20 MB    | 1.01 MB    | 1.01 MB    | 331.91 kB  | 331.56 kB  | echarts.js
 
-6.69 MB    | 2.32 MB    | 2.31 MB    | 492.81 kB  | 488.28 kB  | antd.js   
+6.69 MB    | 2.32 MB    | 2.31 MB    | 492.80 kB  | 488.28 kB  | antd.js   
 
 10.95 MB   | 3.50 MB    | 3.49 MB    | 909.30 kB  | 915.50 kB  | typescript.js