From baaec6020c188da41d7d0653e9191a089ab73467 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Fri, 10 Jan 2025 09:33:09 +0000 Subject: [PATCH] refactor(minifier): remove the buggy `??` transform (#8411) e.g. `(a != null ? a : b);` is not `a ?? b` There are also no unit tests. --- .../peephole_minimize_conditions.rs | 40 ++----------------- tasks/minsize/minsize.snap | 20 +++++----- 2 files changed, 14 insertions(+), 46 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 index e1a8f8f9592dc..9c666314513e9 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs @@ -15,6 +15,7 @@ use crate::{ctx::Ctx, CompressorPass}; /// /// pub struct PeepholeMinimizeConditions { + #[allow(unused)] target: ESTarget, pub(crate) changed: bool, } @@ -81,7 +82,7 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions { if let Some(folded_expr) = match expr { Expression::UnaryExpression(e) => Self::try_minimize_not(e, ctx), Expression::BinaryExpression(e) => Self::try_minimize_binary(e, ctx), - Expression::ConditionalExpression(e) => self.try_minimize_conditional(e, ctx), + Expression::ConditionalExpression(e) => Self::try_minimize_conditional(e, ctx), _ => None, } { *expr = folded_expr; @@ -283,7 +284,6 @@ impl<'a> PeepholeMinimizeConditions { // https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745 fn try_minimize_conditional( - &self, expr: &mut ConditionalExpression<'a>, ctx: &mut TraverseCtx<'a>, ) -> Option> { @@ -589,39 +589,7 @@ impl<'a> PeepholeMinimizeConditions { } } - // Try using the "??" or "?." operators - if let Expression::BinaryExpression(bin_expr) = &mut expr.test { - if bin_expr.operator == BinaryOperator::Equality - || bin_expr.operator == BinaryOperator::Inequality - { - if let Some(check) = { - if bin_expr.left.is_null() { - Some(&bin_expr.right) - } else { - Some(&bin_expr.left) - } - } { - // `a != null ? a : b` -> `a ?? b`` - if check.content_eq(if bin_expr.operator == BinaryOperator::Equality { - &expr.alternate - } else { - &expr.consequent - }) && self.target >= ESTarget::ES2020 - // TODO: this is probably a but too aggressive - && matches!(check, Expression::Identifier(_)) - { - return Some(ctx.ast.expression_logical( - SPAN, - ctx.ast.move_expression(&mut expr.consequent), - LogicalOperator::Coalesce, - ctx.ast.move_expression(&mut expr.alternate), - )); - } - - // TODO: `a != null ? a.b.c[d](e) : undefined` -> `a?.b.c[d](e)`` - } - } - } + // TODO: Try using the "??" or "?." operators // Non esbuild optimizations @@ -2023,8 +1991,8 @@ mod test { test("var a; a ? b(c, d) : b(e, d)", "var a; b(a ? c : e, d)"); 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 ?? b"); test("a() != null ? a() : b", "a() == null ? b : a()"); + // test("a != null ? a : b", "a ?? b"); // test("a != null ? a.b.c[d](e) : undefined", "a?.b.c[d](e)"); } diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index da530171efb24..b1efa08baf988 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.71 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js -173.90 kB | 59.77 kB | 59.82 kB | 19.40 kB | 19.33 kB | moment.js +173.90 kB | 59.79 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js -287.63 kB | 90.03 kB | 90.07 kB | 32.02 kB | 31.95 kB | jquery.js +287.63 kB | 90.09 kB | 90.07 kB | 32.03 kB | 31.95 kB | jquery.js -342.15 kB | 118.10 kB | 118.14 kB | 44.43 kB | 44.37 kB | vue.js +342.15 kB | 118.11 kB | 118.14 kB | 44.44 kB | 44.37 kB | vue.js -544.10 kB | 71.74 kB | 72.48 kB | 26.16 kB | 26.20 kB | lodash.js +544.10 kB | 71.76 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js -555.77 kB | 272.92 kB | 270.13 kB | 90.86 kB | 90.80 kB | d3.js +555.77 kB | 273.21 kB | 270.13 kB | 90.92 kB | 90.80 kB | d3.js -1.01 MB | 460.13 kB | 458.89 kB | 126.75 kB | 126.71 kB | bundle.min.js +1.01 MB | 460.18 kB | 458.89 kB | 126.76 kB | 126.71 kB | bundle.min.js 1.25 MB | 652.85 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js -2.14 MB | 726.02 kB | 724.14 kB | 180.01 kB | 181.07 kB | victory.js +2.14 MB | 726.27 kB | 724.14 kB | 180.09 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 331.67 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 331.78 kB | 331.56 kB | echarts.js -6.69 MB | 2.32 MB | 2.31 MB | 492.60 kB | 488.28 kB | antd.js +6.69 MB | 2.32 MB | 2.31 MB | 492.64 kB | 488.28 kB | antd.js -10.95 MB | 3.49 MB | 3.49 MB | 907.27 kB | 915.50 kB | typescript.js +10.95 MB | 3.49 MB | 3.49 MB | 907.46 kB | 915.50 kB | typescript.js