Skip to content

Commit

Permalink
refactor(minifier): clean up try_minimize_conditional (#8660)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Boshen
Boshen committed Jan 22, 2025
1 parent ae8db53 commit 3802d28
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 120 deletions.
211 changes: 102 additions & 109 deletions crates/oxc_minifier/src/peephole/minimize_conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,130 +353,108 @@ impl<'a> PeepholeOptimizations {
}
}

// https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745
// <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745>
fn try_minimize_conditional(
expr: &mut ConditionalExpression<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
// `(a, b) ? c : d` -> `a, b ? c : d`
if let Expression::SequenceExpression(sequence_expr) = &mut expr.test {
if sequence_expr.expressions.len() > 1 {
let span = expr.span();
let mut sequence = ctx.ast.move_expression(&mut expr.test);
let Expression::SequenceExpression(sequence_expr) = &mut sequence else {
unreachable!()
};
let test = sequence_expr.expressions.pop().unwrap();
sequence_expr.expressions.push(ctx.ast.expression_conditional(
span,
test,
ctx.ast.move_expression(&mut expr.consequent),
ctx.ast.move_expression(&mut expr.alternate),
));
return Some(sequence);
match &mut expr.test {
// `x != y ? b : c` -> `x == y ? c : b`
Expression::BinaryExpression(test_expr) => {
if matches!(
test_expr.operator,
BinaryOperator::Inequality | BinaryOperator::StrictInequality
) {
test_expr.operator = test_expr.operator.equality_inverse_operator().unwrap();
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(
ctx.ast.expression_conditional(expr.span, test, alternate, consequent),
);
}
}
}

// `!a ? b : c` -> `a ? c : b`
if let Expression::UnaryExpression(test_expr) = &mut expr.test {
if test_expr.operator.is_not()
// Skip `!!!a`
&& !matches!(test_expr.argument, Expression::UnaryExpression(_))
{
let test = ctx.ast.move_expression(&mut test_expr.argument);
let consequent = ctx.ast.move_expression(&mut expr.consequent);
let alternate = ctx.ast.move_expression(&mut expr.alternate);
return Some(
ctx.ast.expression_conditional(expr.span, test, alternate, consequent),
);
// "(a, b) ? c : d" => "a, b ? c : d"
Expression::SequenceExpression(sequence_expr) => {
if sequence_expr.expressions.len() > 1 {
let span = expr.span();
let mut sequence = ctx.ast.move_expression(&mut expr.test);
let Expression::SequenceExpression(sequence_expr) = &mut sequence else {
unreachable!()
};
let mut cond_expr = ctx.ast.conditional_expression(
span,
sequence_expr.expressions.pop().unwrap(),
ctx.ast.move_expression(&mut expr.consequent),
ctx.ast.move_expression(&mut expr.alternate),
);
let expr =
Self::try_minimize_conditional(&mut cond_expr, ctx).unwrap_or_else(|| {
Expression::ConditionalExpression(ctx.ast.alloc(cond_expr))
});
sequence_expr.expressions.push(expr);
return Some(sequence);
}
}
}

// `x != y ? b : c` -> `x == y ? c : b`
if let Expression::BinaryExpression(test_expr) = &mut expr.test {
if matches!(
test_expr.operator,
BinaryOperator::Inequality | BinaryOperator::StrictInequality
) {
test_expr.operator = test_expr.operator.equality_inverse_operator().unwrap();
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(
ctx.ast.expression_conditional(expr.span, test, alternate, consequent),
);
// "!a ? b : c" => "a ? c : b"
Expression::UnaryExpression(test_expr) => {
if test_expr.operator.is_not() {
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(
ctx.ast.expression_conditional(expr.span, test, consequent, alternate),
);
}
}
// "a ? a : b" => "a || b"
Expression::Identifier(id) => {
if let Expression::Identifier(id2) = &expr.consequent {
if id.name == id2.name {
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.move_expression(&mut expr.test),
LogicalOperator::Or,
ctx.ast.move_expression(&mut expr.alternate),
));
}
}
// "a ? b : a" => "a && b"
if let Expression::Identifier(id2) = &expr.alternate {
if id.name == id2.name {
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.move_expression(&mut expr.test),
LogicalOperator::And,
ctx.ast.move_expression(&mut expr.consequent),
));
}
}
}
_ => {}
}

// TODO: `/* @__PURE__ */ a() ? b : b` -> `b`

// `a ? b : b` -> `a, b`
if expr.alternate.content_eq(&expr.consequent) {
let expressions = ctx.ast.vec_from_array([
ctx.ast.move_expression(&mut expr.test),
ctx.ast.move_expression(&mut expr.consequent),
]);
return Some(ctx.ast.expression_sequence(expr.span, expressions));
}

// `a ? true : false` -> `!!a`
// `a ? false : true` -> `!a`
if let (
Expression::Identifier(_),
Expression::BooleanLiteral(consequent_lit),
Expression::BooleanLiteral(alternate_lit),
) = (&expr.test, &expr.consequent, &expr.alternate)
// "a ? true : false" => "!!a"
// "a ? false : true" => "!a"
if let (Expression::BooleanLiteral(left), Expression::BooleanLiteral(right)) =
(&expr.consequent, &expr.alternate)
{
match (consequent_lit.value, alternate_lit.value) {
let op = UnaryOperator::LogicalNot;
match (left.value, right.value) {
(false, true) => {
let ident = ctx.ast.move_expression(&mut expr.test);
return Some(ctx.ast.expression_unary(
expr.span,
UnaryOperator::LogicalNot,
ident,
));
return Some(ctx.ast.expression_unary(expr.span, op, ident));
}
(true, false) => {
let ident = ctx.ast.move_expression(&mut expr.test);
return Some(ctx.ast.expression_unary(
expr.span,
UnaryOperator::LogicalNot,
ctx.ast.expression_unary(expr.span, UnaryOperator::LogicalNot, ident),
));
let unary = ctx.ast.expression_unary(expr.span, op, ident);
return Some(ctx.ast.expression_unary(expr.span, op, unary));
}
_ => {}
}
}

// `a ? a : b` -> `a || b`
if let (Expression::Identifier(test_ident), Expression::Identifier(consequent_ident)) =
(&expr.test, &expr.consequent)
{
if test_ident.name == consequent_ident.name {
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.move_expression(&mut expr.test),
LogicalOperator::Or,
ctx.ast.move_expression(&mut expr.alternate),
));
}
}

// `a ? b : a` -> `a && b`
if let (Expression::Identifier(test_ident), Expression::Identifier(alternate_ident)) =
(&expr.test, &expr.alternate)
{
if test_ident.name == alternate_ident.name {
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.move_expression(&mut expr.test),
LogicalOperator::And,
ctx.ast.move_expression(&mut expr.consequent),
));
}
}

// `a ? b ? c : d : d` -> `a && b ? c : d`
// "a ? b ? c : d : d" => "a && b ? c : d"
if let Expression::ConditionalExpression(consequent) = &mut expr.consequent {
if consequent.alternate.content_eq(&expr.alternate) {
return Some(ctx.ast.expression_conditional(
Expand All @@ -493,7 +471,7 @@ impl<'a> PeepholeOptimizations {
}
}

// `a ? b : c ? b : d` -> `a || c ? b : d`
// "a ? b : c ? b : d" => "a || c ? b : d"
if let Expression::ConditionalExpression(alternate) = &mut expr.alternate {
if alternate.consequent.content_eq(&expr.consequent) {
return Some(ctx.ast.expression_conditional(
Expand All @@ -510,7 +488,7 @@ impl<'a> PeepholeOptimizations {
}
}

// `a ? c : (b, c)` -> `(a || b), c`
// "a ? c : (b, c)" => "(a || b), c"
if let Expression::SequenceExpression(alternate) = &mut expr.alternate {
if alternate.expressions.len() == 2
&& alternate.expressions[1].content_eq(&expr.consequent)
Expand All @@ -530,7 +508,7 @@ impl<'a> PeepholeOptimizations {
}
}

// `a ? (b, c) : c` -> `(a && b), c`
// "a ? (b, c) : c" => "(a && b), c"
if let Expression::SequenceExpression(consequent) = &mut expr.consequent {
if consequent.expressions.len() == 2
&& consequent.expressions[1].content_eq(&expr.alternate)
Expand All @@ -550,7 +528,7 @@ impl<'a> PeepholeOptimizations {
}
}

// `a ? b || c : c` => "(a && b) || c"
// "a ? b || c : c" => "(a && b) || c"
if let Expression::LogicalExpression(logical_expr) = &mut expr.consequent {
if logical_expr.operator == LogicalOperator::Or
&& logical_expr.right.content_eq(&expr.alternate)
Expand All @@ -569,7 +547,7 @@ impl<'a> PeepholeOptimizations {
}
}

// `a ? c : b && c` -> `(a || b) && c`
// "a ? c : b && c" => "(a || b) && c"
if let Expression::LogicalExpression(logical_expr) = &mut expr.alternate {
if logical_expr.operator == LogicalOperator::And
&& logical_expr.right.content_eq(&expr.consequent)
Expand Down Expand Up @@ -715,6 +693,21 @@ impl<'a> PeepholeOptimizations {
));
}

if expr.alternate.content_eq(&expr.consequent) {
// TODO:
// "/* @__PURE__ */ a() ? b : b" => "b"
// if ctx.ExprCanBeRemovedIfUnused(test) {
// return yes
// }

// "a ? b : b" => "a, b"
let expressions = ctx.ast.vec_from_array([
ctx.ast.move_expression(&mut expr.test),
ctx.ast.move_expression(&mut expr.consequent),
]);
return Some(ctx.ast.expression_sequence(expr.span, expressions));
}

None
}

Expand Down
8 changes: 4 additions & 4 deletions crates/oxc_minifier/src/peephole/replace_known_methods.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,16 +333,16 @@ impl<'a> PeepholeOptimizations {
/// `[].concat(a).concat(b)` -> `[].concat(a, b)`
/// `"".concat(a).concat(b)` -> `"".concat(a, b)`
fn try_fold_concat_chain(&mut self, node: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) {
if matches!(ctx.parent(), Ancestor::StaticMemberExpressionObject(_)) {
return;
}

let original_span = if let Expression::CallExpression(root_call_expr) = node {
root_call_expr.span
} else {
return;
};

if matches!(ctx.parent(), Ancestor::StaticMemberExpressionObject(_)) {
return;
}

let mut current_node: &mut Expression = node;
let mut collected_arguments = ctx.ast.vec();
let new_root_callee: &mut Expression<'a>;
Expand Down
14 changes: 7 additions & 7 deletions tasks/minsize/minsize.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ Original | minified | minified | gzip | gzip | Fixture

342.15 kB | 118.19 kB | 118.14 kB | 44.45 kB | 44.37 kB | vue.js

544.10 kB | 71.74 kB | 72.48 kB | 26.14 kB | 26.20 kB | lodash.js
544.10 kB | 71.73 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js

555.77 kB | 272.89 kB | 270.13 kB | 90.90 kB | 90.80 kB | d3.js

1.01 MB | 460.18 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js
1.01 MB | 460.17 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js

1.25 MB | 652.90 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js
1.25 MB | 652.85 kB | 646.76 kB | 163.53 kB | 163.73 kB | three.js

2.14 MB | 724.01 kB | 724.14 kB | 179.93 kB | 181.07 kB | victory.js
2.14 MB | 724.00 kB | 724.14 kB | 179.93 kB | 181.07 kB | victory.js

3.20 MB | 1.01 MB | 1.01 MB | 332.01 kB | 331.56 kB | echarts.js
3.20 MB | 1.01 MB | 1.01 MB | 332.00 kB | 331.56 kB | echarts.js

6.69 MB | 2.31 MB | 2.31 MB | 491.97 kB | 488.28 kB | antd.js
6.69 MB | 2.31 MB | 2.31 MB | 491.95 kB | 488.28 kB | antd.js

10.95 MB | 3.48 MB | 3.49 MB | 905.34 kB | 915.50 kB | typescript.js
10.95 MB | 3.48 MB | 3.49 MB | 905.35 kB | 915.50 kB | typescript.js

0 comments on commit 3802d28

Please sign in to comment.