From 72d74a2fdc8a1f0faa912984ec5ad8e87a9a8e9f Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Fri, 31 Jan 2025 06:47:47 +0000 Subject: [PATCH] feat(minifier): compress `a != null ? a : b` into `a ?? b` (#8801) It seems this was implemented in past by #8352 and was reverted in #8411. I'm not sure what was buggy, but one difference with this PR is that the previous PR doesn't check whether the identifier is a global reference or not. --- .../src/peephole/minimize_conditions.rs | 88 +++++++++++++++---- .../src/peephole/minimize_statements.rs | 11 +-- crates/oxc_minifier/tests/peephole/esbuild.rs | 22 ++--- tasks/minsize/minsize.snap | 20 ++--- 4 files changed, 95 insertions(+), 46 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_conditions.rs b/crates/oxc_minifier/src/peephole/minimize_conditions.rs index 0c322898841e8..53a8c70929309 100644 --- a/crates/oxc_minifier/src/peephole/minimize_conditions.rs +++ b/crates/oxc_minifier/src/peephole/minimize_conditions.rs @@ -46,7 +46,7 @@ impl<'a> PeepholeOptimizations { if let Some(folded_stmt) = match stmt { // If the condition is a literal, we'll let other optimizations try to remove useless code. - Statement::IfStatement(_) => Self::try_minimize_if(stmt, ctx), + Statement::IfStatement(_) => self.try_minimize_if(stmt, ctx), _ => None, } { *stmt = folded_stmt; @@ -73,7 +73,7 @@ impl<'a> PeepholeOptimizations { if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, ctx) { local_change = true; } - Self::try_minimize_conditional(logical_expr, ctx) + self.try_minimize_conditional(logical_expr, ctx) } Expression::AssignmentExpression(e) => { if self.try_compress_normal_assignment_to_combined_logical_assignment(e, ctx) { @@ -146,7 +146,7 @@ impl<'a> PeepholeOptimizations { } /// `MangleIf`: - fn try_minimize_if(stmt: &mut Statement<'a>, ctx: Ctx<'a, '_>) -> Option> { + fn try_minimize_if(&self, stmt: &mut Statement<'a>, ctx: Ctx<'a, '_>) -> Option> { let Statement::IfStatement(if_stmt) = stmt else { unreachable!() }; // `if (x) foo()` -> `x && foo()` @@ -221,7 +221,7 @@ impl<'a> PeepholeOptimizations { let consequent = Self::get_block_expression(&mut if_stmt.consequent, ctx); let else_branch = if_stmt.alternate.as_mut().unwrap(); let alternate = Self::get_block_expression(else_branch, ctx); - let expr = Self::minimize_conditional( + let expr = self.minimize_conditional( if_stmt.span, test, consequent, @@ -268,6 +268,7 @@ impl<'a> PeepholeOptimizations { } pub fn minimize_conditional( + &self, span: Span, test: Expression<'a>, consequent: Expression<'a>, @@ -275,12 +276,13 @@ impl<'a> PeepholeOptimizations { ctx: Ctx<'a, '_>, ) -> Expression<'a> { let mut cond_expr = ctx.ast.conditional_expression(span, test, consequent, alternate); - Self::try_minimize_conditional(&mut cond_expr, ctx) + self.try_minimize_conditional(&mut cond_expr, ctx) .unwrap_or_else(|| Expression::ConditionalExpression(ctx.ast.alloc(cond_expr))) } // `MangleIfExpr`: fn try_minimize_conditional( + &self, expr: &mut ConditionalExpression<'a>, ctx: Ctx<'a, '_>, ) -> Option> { @@ -293,7 +295,7 @@ impl<'a> PeepholeOptimizations { let Expression::SequenceExpression(sequence_expr) = &mut sequence else { unreachable!() }; - let expr = Self::minimize_conditional( + let expr = self.minimize_conditional( span, sequence_expr.expressions.pop().unwrap(), ctx.ast.move_expression(&mut expr.consequent), @@ -310,9 +312,9 @@ impl<'a> PeepholeOptimizations { let test = ctx.ast.move_expression(&mut test_expr.argument); let consequent = ctx.ast.move_expression(&mut expr.alternate); let alternate = ctx.ast.move_expression(&mut expr.consequent); - return Some(Self::minimize_conditional( - expr.span, test, consequent, alternate, ctx, - )); + return Some( + self.minimize_conditional(expr.span, test, consequent, alternate, ctx), + ); } } Expression::Identifier(id) => { @@ -351,9 +353,9 @@ impl<'a> PeepholeOptimizations { let test = ctx.ast.move_expression(&mut expr.test); let consequent = ctx.ast.move_expression(&mut expr.consequent); let alternate = ctx.ast.move_expression(&mut expr.alternate); - return Some(Self::minimize_conditional( - expr.span, test, alternate, consequent, ctx, - )); + return Some( + self.minimize_conditional(expr.span, test, alternate, consequent, ctx), + ); } } _ => {} @@ -555,7 +557,7 @@ impl<'a> PeepholeOptimizations { let alternate_first_arg = ctx.ast.move_expression(alternate.arguments[0].to_expression_mut()); let mut args = std::mem::replace(&mut consequent.arguments, ctx.ast.vec()); - let cond_expr = Self::minimize_conditional( + let cond_expr = self.minimize_conditional( expr.test.span(), ctx.ast.move_expression(&mut expr.test), consequent_first_arg, @@ -569,11 +571,52 @@ impl<'a> PeepholeOptimizations { } // Not part of esbuild - if let Some(e) = Self::try_merge_conditional_expression_inside(expr, ctx) { + if let Some(e) = self.try_merge_conditional_expression_inside(expr, ctx) { return Some(e); } - // TODO: Try using the "??" or "?." operators + // Try using the "??" or "?." operators + if self.target >= ESTarget::ES2020 { + if let Expression::BinaryExpression(test_binary) = &expr.test { + if let Some(is_negate) = match test_binary.operator { + BinaryOperator::Inequality => Some(true), + BinaryOperator::Equality => Some(false), + _ => None, + } { + // a == null / a != null + if let Some((id_expr, ())) = Self::commutative_pair( + (&test_binary.left, &test_binary.right), + |a| { + if let Expression::Identifier(id) = a { + (!ctx.is_global_reference(id)).then_some(a) + } else { + None + } + }, + |b| b.is_null().then_some(()), + ) { + // `a == null ? b : a` -> `a ?? b` + // `a != null ? a : b` -> `a ?? b` + let target_expr = + if is_negate { &mut expr.consequent } else { &mut expr.alternate }; + if id_expr.content_eq(target_expr) { + return Some(ctx.ast.expression_logical( + expr.span, + ctx.ast.move_expression(target_expr), + LogicalOperator::Coalesce, + ctx.ast.move_expression(if is_negate { + &mut expr.alternate + } else { + &mut expr.consequent + }), + )); + } + + // TODO: try using optional chaining + } + } + } + } if ctx.expr_eq(&expr.alternate, &expr.consequent) { // TODO: @@ -639,6 +682,7 @@ impl<'a> PeepholeOptimizations { /// /// - `x ? a = 0 : a = 1` -> `a = x ? 0 : 1` fn try_merge_conditional_expression_inside( + &self, expr: &mut ConditionalExpression<'a>, ctx: Ctx<'a, '_>, ) -> Option> { @@ -661,7 +705,7 @@ impl<'a> PeepholeOptimizations { { return None; } - let cond_expr = Self::minimize_conditional( + let cond_expr = self.minimize_conditional( expr.span, ctx.ast.move_expression(&mut expr.test), ctx.ast.move_expression(&mut consequent.right), @@ -1151,6 +1195,14 @@ mod test { }; use oxc_syntax::es_target::ESTarget; + fn test_es2019(source_text: &str, expected: &str) { + let target = ESTarget::ES2019; + assert_eq!( + run(source_text, Some(CompressOptions { target, ..CompressOptions::default() })), + run(expected, None) + ); + } + /** Check that removing blocks with 1 child works */ #[test] fn test_fold_one_child_blocks() { @@ -2264,7 +2316,9 @@ mod test { test("var a; a ? b(...c) : b(...e)", "var a; b(...a ? c : e)"); test("var a; a ? b(c) : b(e)", "var a; b(a ? c : e)"); test("a() != null ? a() : b", "a() == null ? b : a()"); - // test("a != null ? a : b", "a ?? b"); + test("var a; a != null ? a : b", "var a; a ?? b"); + test("a != null ? a : b", "a == null ? b : a"); // accessing global `a` may have a getter with side effects + test_es2019("var a; a != null ? a : b", "var a; a == null ? b : a"); // test("a != null ? a.b.c[d](e) : undefined", "a?.b.c[d](e)"); test("cmp !== 0 ? cmp : (bar, cmp);", "cmp === 0 && bar, cmp;"); test("cmp === 0 ? cmp : (bar, cmp);", "cmp === 0 || bar, cmp;"); diff --git a/crates/oxc_minifier/src/peephole/minimize_statements.rs b/crates/oxc_minifier/src/peephole/minimize_statements.rs index 0791e2093d25d..6d3deb5fc85c7 100644 --- a/crates/oxc_minifier/src/peephole/minimize_statements.rs +++ b/crates/oxc_minifier/src/peephole/minimize_statements.rs @@ -143,17 +143,12 @@ impl<'a> PeepholeOptimizations { { // "if (a, b) return c; return d;" => "return a, b ? c : d;" let test = sequence_expr.expressions.pop().unwrap(); - let mut b = Self::minimize_conditional( - prev_if.span, - test, - left, - right, - ctx, - ); + let mut b = + self.minimize_conditional(prev_if.span, test, left, right, ctx); Self::join_sequence(&mut prev_if.test, &mut b, ctx) } else { // "if (a) return b; return c;" => "return a ? b : c;" - let mut expr = Self::minimize_conditional( + let mut expr = self.minimize_conditional( prev_if.span, ctx.ast.move_expression(&mut prev_if.test), left, diff --git a/crates/oxc_minifier/tests/peephole/esbuild.rs b/crates/oxc_minifier/tests/peephole/esbuild.rs index 42f99f17b3813..ce301969483f3 100644 --- a/crates/oxc_minifier/tests/peephole/esbuild.rs +++ b/crates/oxc_minifier/tests/peephole/esbuild.rs @@ -592,12 +592,12 @@ fn js_parser_test() { test("a ? c : b || c", "a ? c : b || c;"); test("a = b == null ? c : b", "a = b == null ? c : b;"); test("a = b != null ? b : c", "a = b == null ? c : b;"); - // test("let b; a = b == null ? c : b", "let b;a = b ?? c;"); - // test("let b; a = b != null ? b : c", "let b;a = b ?? c;"); + test("let b; a = b == null ? c : b", "let b;a = b ?? c;"); + test("let b; a = b != null ? b : c", "let b;a = b ?? c;"); test("let b; a = b == null ? b : c", "let b;a = b == null ? b : c;"); - // test("let b; a = b != null ? c : b", "let b;a = b != null ? c : b;"); - // test("let b; a = null == b ? c : b", "let b;a = b ?? c;"); - // test("let b; a = null != b ? b : c", "let b;a = b ?? c;"); + test("let b; a = b != null ? c : b", "let b;a = b == null ? b : c;"); + test("let b; a = null == b ? c : b", "let b;a = b ?? c;"); + test("let b; a = null != b ? b : c", "let b;a = b ?? c;"); test("let b; a = null == b ? b : c", "let b;a = b == null ? b : c;"); // test("let b; a = null != b ? c : b", "let b;a = b != null ? c : b;"); test("let b; a = b.x == null ? c : b.x", "let b;a = b.x == null ? c : b.x;"); @@ -608,8 +608,8 @@ fn js_parser_test() { // test("let b; a = b !== null ? b : c", "let b;a = b !== null ? b : c;"); test("let b; a = null === b ? c : b", "let b;a = b === null ? c : b;"); // test("let b; a = null !== b ? b : c", "let b;a = b !== null ? b : c;"); - // test("let b; a = null === b || b === undefined ? c : b", "let b;a = b ?? c;"); - // test("let b; a = b !== undefined && b !== null ? b : c", "let b;a = b ?? c;"); + test("let b; a = null === b || b === undefined ? c : b", "let b;a = b ?? c;"); + test("let b; a = b !== undefined && b !== null ? b : c", "let b;a = b ?? c;"); // test("a(b ? 0 : 0)", "a((b, 0));"); // test("a(b ? +0 : -0)", "a(b ? 0 : -0);"); // test("a(b ? +0 : 0)", "a((b, 0));"); @@ -639,10 +639,10 @@ fn js_parser_test() { test("return a ?? ((b ?? c) ?? (d ?? e))", "return a ?? b ?? c ?? d ?? e;"); test("if (a) if (b) if (c) d", "a && b && c && d;"); test("if (!a) if (!b) if (!c) d", "a || b || c || d;"); - // test( - // "let a, b, c; return a != null ? a : b != null ? b : c", - // "let a, b, c;return a ?? b ?? c;", - // ); + test( + "let a, b, c; return a != null ? a : b != null ? b : c", + "let a, b, c;return a ?? b ?? c;", + ); test("if (a) return c; if (b) return d;", "if (a) return c;if (b) return d;"); // test("if (a) return c; if (b) return c;", "if (a || b) return c;"); } diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index a7fb93014420a..5bd6c27a469e0 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -3,25 +3,25 @@ Original | minified | minified | gzip | gzip | Fixture ------------------------------------------------------------------------------------- 72.14 kB | 23.56 kB | 23.70 kB | 8.51 kB | 8.54 kB | react.development.js -173.90 kB | 59.57 kB | 59.82 kB | 19.19 kB | 19.33 kB | moment.js +173.90 kB | 59.55 kB | 59.82 kB | 19.19 kB | 19.33 kB | moment.js -287.63 kB | 89.53 kB | 90.07 kB | 30.95 kB | 31.95 kB | jquery.js +287.63 kB | 89.49 kB | 90.07 kB | 30.95 kB | 31.95 kB | jquery.js -342.15 kB | 117.69 kB | 118.14 kB | 43.57 kB | 44.37 kB | vue.js +342.15 kB | 117.68 kB | 118.14 kB | 43.56 kB | 44.37 kB | vue.js -544.10 kB | 71.44 kB | 72.48 kB | 25.87 kB | 26.20 kB | lodash.js +544.10 kB | 71.43 kB | 72.48 kB | 25.87 kB | 26.20 kB | lodash.js -555.77 kB | 271.45 kB | 270.13 kB | 88.38 kB | 90.80 kB | d3.js +555.77 kB | 271.25 kB | 270.13 kB | 88.35 kB | 90.80 kB | d3.js -1.01 MB | 441.00 kB | 458.89 kB | 122.51 kB | 126.71 kB | bundle.min.js +1.01 MB | 440.96 kB | 458.89 kB | 122.50 kB | 126.71 kB | bundle.min.js 1.25 MB | 650.36 kB | 646.76 kB | 161.02 kB | 163.73 kB | three.js -2.14 MB | 718.80 kB | 724.14 kB | 162.18 kB | 181.07 kB | victory.js +2.14 MB | 718.64 kB | 724.14 kB | 162.14 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 324.37 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 324.31 kB | 331.56 kB | echarts.js -6.69 MB | 2.30 MB | 2.31 MB | 469.25 kB | 488.28 kB | antd.js +6.69 MB | 2.30 MB | 2.31 MB | 469.23 kB | 488.28 kB | antd.js -10.95 MB | 3.37 MB | 3.49 MB | 864.67 kB | 915.50 kB | typescript.js +10.95 MB | 3.37 MB | 3.49 MB | 864.49 kB | 915.50 kB | typescript.js