Skip to content

Commit

Permalink
feat(minifier): compress a != null ? a : b into a ?? b (#8801)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sapphi-red
sapphi-red committed Jan 31, 2025
1 parent a861d93 commit 72d74a2
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 46 deletions.
88 changes: 71 additions & 17 deletions crates/oxc_minifier/src/peephole/minimize_conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -146,7 +146,7 @@ impl<'a> PeepholeOptimizations {
}

/// `MangleIf`: <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_parser.go#L9190>
fn try_minimize_if(stmt: &mut Statement<'a>, ctx: Ctx<'a, '_>) -> Option<Statement<'a>> {
fn try_minimize_if(&self, stmt: &mut Statement<'a>, ctx: Ctx<'a, '_>) -> Option<Statement<'a>> {
let Statement::IfStatement(if_stmt) = stmt else { unreachable!() };

// `if (x) foo()` -> `x && foo()`
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -268,19 +268,21 @@ impl<'a> PeepholeOptimizations {
}

pub fn minimize_conditional(
&self,
span: Span,
test: Expression<'a>,
consequent: Expression<'a>,
alternate: Expression<'a>,
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`: <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: Ctx<'a, '_>,
) -> Option<Expression<'a>> {
Expand All @@ -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),
Expand All @@ -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) => {
Expand Down Expand Up @@ -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),
);
}
}
_ => {}
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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<Expression<'a>> {
Expand All @@ -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),
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;");
Expand Down
11 changes: 3 additions & 8 deletions crates/oxc_minifier/src/peephole/minimize_statements.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 11 additions & 11 deletions crates/oxc_minifier/tests/peephole/esbuild.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;");
Expand All @@ -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));");
Expand Down Expand Up @@ -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;");
}
Expand Down
20 changes: 10 additions & 10 deletions tasks/minsize/minsize.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 72d74a2

Please sign in to comment.