Skip to content

Commit

Permalink
fix(minifier): remove "non esbuild optimizations" which is incorrect (#…
Browse files Browse the repository at this point in the history 
…8668)
  • Loading branch information
@Boshen
Boshen committed Jan 23, 2025
1 parent 4ae568e commit ba201a6
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 83 deletions.
102 changes: 26 additions & 76 deletions crates/oxc_minifier/src/peephole/minimize_conditions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,16 @@ 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 = ctx.ast.expression_conditional(
let mut cond_expr = ctx.ast.conditional_expression(
if_stmt.span,
test,
consequent,
alternate,
);
let expr = Self::try_minimize_conditional(&mut cond_expr, ctx)
.unwrap_or_else(|| {
Expression::ConditionalExpression(ctx.ast.alloc(cond_expr))
});
return Some(ctx.ast.statement_expression(if_stmt.span, expr));
}
}
Expand Down Expand Up @@ -281,12 +285,14 @@ impl<'a> PeepholeOptimizations {
let mut if_stmt = if_stmt.unbox();
let consequent = Self::get_block_return_expression(&mut if_stmt.consequent, ctx);
let alternate = Self::take_return_argument(&mut stmts[i + 1], ctx);
let argument = ctx.ast.expression_conditional(
let mut cond_expr = ctx.ast.conditional_expression(
if_stmt.span,
if_stmt.test,
consequent,
alternate,
);
let argument = Self::try_minimize_conditional(&mut cond_expr, ctx)
.unwrap_or_else(|| Expression::ConditionalExpression(ctx.ast.alloc(cond_expr)));
stmts[i] = ctx.ast.statement_return(if_stmt.span, Some(argument));
*changed = true;
break;
Expand Down Expand Up @@ -359,21 +365,6 @@ impl<'a> PeepholeOptimizations {
ctx: &mut TraverseCtx<'a>,
) -> Option<Expression<'a>> {
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 : d" => "a, b ? c : d"
Expression::SequenceExpression(sequence_expr) => {
if sequence_expr.expressions.len() > 1 {
Expand Down Expand Up @@ -431,6 +422,21 @@ impl<'a> PeepholeOptimizations {
}
}
}
// `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),
);
}
}
_ => {}
}

Expand Down Expand Up @@ -637,62 +643,6 @@ impl<'a> PeepholeOptimizations {

// TODO: Try using the "??" or "?." operators

// Non esbuild optimizations

// `x ? true : y` -> `x || y`
// `x ? false : y` -> `!x && y`
if let (Expression::Identifier(_), Expression::BooleanLiteral(consequent_lit), _) =
(&expr.test, &expr.consequent, &expr.alternate)
{
if consequent_lit.value {
let ident = ctx.ast.move_expression(&mut expr.test);
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.expression_unary(
ident.span(),
UnaryOperator::LogicalNot,
ctx.ast.expression_unary(ident.span(), UnaryOperator::LogicalNot, ident),
),
LogicalOperator::Or,
ctx.ast.move_expression(&mut expr.alternate),
));
}
let ident = ctx.ast.move_expression(&mut expr.test);
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.expression_unary(expr.span, UnaryOperator::LogicalNot, ident),
LogicalOperator::And,
ctx.ast.move_expression(&mut expr.alternate),
));
}

// `x ? y : true` -> `!x || y`
// `x ? y : false` -> `x && y`
if let (Expression::Identifier(_), _, Expression::BooleanLiteral(alternate_lit)) =
(&expr.test, &expr.consequent, &expr.alternate)
{
if alternate_lit.value {
let ident = ctx.ast.move_expression(&mut expr.test);
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.expression_unary(expr.span, UnaryOperator::LogicalNot, ident),
LogicalOperator::Or,
ctx.ast.move_expression(&mut expr.consequent),
));
}
let ident = ctx.ast.move_expression(&mut expr.test);
return Some(ctx.ast.expression_logical(
expr.span,
ctx.ast.expression_unary(
expr.span,
UnaryOperator::LogicalNot,
ctx.ast.expression_unary(ident.span(), UnaryOperator::LogicalNot, ident),
),
LogicalOperator::And,
ctx.ast.move_expression(&mut expr.consequent),
));
}

if expr.alternate.content_eq(&expr.consequent) {
// TODO:
// "/* @__PURE__ */ a() ? b : b" => "b"
Expand Down Expand Up @@ -1163,11 +1113,11 @@ mod test {
test("(x || false) && y()", "x && y()");

test("let x = foo ? true : false", "let x = !!foo");
test("let x = foo ? true : bar", "let x = !!foo || bar");
test("let x = foo ? bar : false", "let x = !!foo && bar");
test("let x = foo ? true : bar", "let x = foo ? !0 : bar");
test("let x = foo ? bar : false", "let x = foo ? bar : !1");
test("function x () { return a ? true : false }", "function x() { return !!a }");
test("function x () { return a ? false : true }", "function x() { return !a }");
test("function x () { return a ? true : b }", "function x() { return !!a || b }");
test("function x () { return a ? true : b }", "function x() { return a ? !0 : b }");
// can't be minified e.g. `a = ''` would return `''`
test("function x() { return a && true }", "function x() { return a && !0 }");

Expand Down
1 change: 1 addition & 0 deletions crates/oxc_minifier/src/peephole/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl<'a> PeepholeOptimizations {
*current_changed
}

#[inline]
fn is_prev_function_changed(&self) -> bool {
let (_, prev_changed, _) = self.current_function.last();
*prev_changed
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 @@ -5,23 +5,23 @@ Original | minified | minified | gzip | gzip | Fixture

173.90 kB | 59.79 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js

287.63 kB | 90.08 kB | 90.07 kB | 32.02 kB | 31.95 kB | jquery.js
287.63 kB | 90.08 kB | 90.07 kB | 32.03 kB | 31.95 kB | jquery.js

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

544.10 kB | 71.73 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js
544.10 kB | 71.73 kB | 72.48 kB | 26.14 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.17 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js
1.01 MB | 460.16 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js

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

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

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

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

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

0 comments on commit ba201a6

Please sign in to comment.