Skip to content

Commit 72d74a2

Browse files
sapphi-redsapphi-red
committed
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.
1 parent a861d93 commit 72d74a2

File tree

4 files changed

+95
-46
lines changed

4 files changed

+95
-46
lines changed

‎crates/oxc_minifier/src/peephole/minimize_conditions.rs

+71-17
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ impl<'a> PeepholeOptimizations {
4646

4747
if let Some(folded_stmt) = match stmt {
4848
// If the condition is a literal, we'll let other optimizations try to remove useless code.
49-
Statement::IfStatement(_) => Self::try_minimize_if(stmt, ctx),
49+
Statement::IfStatement(_) => self.try_minimize_if(stmt, ctx),
5050
_ => None,
5151
} {
5252
*stmt = folded_stmt;
@@ -73,7 +73,7 @@ impl<'a> PeepholeOptimizations {
7373
if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, ctx) {
7474
local_change = true;
7575
}
76-
Self::try_minimize_conditional(logical_expr, ctx)
76+
self.try_minimize_conditional(logical_expr, ctx)
7777
}
7878
Expression::AssignmentExpression(e) => {
7979
if self.try_compress_normal_assignment_to_combined_logical_assignment(e, ctx) {
@@ -146,7 +146,7 @@ impl<'a> PeepholeOptimizations {
146146
}
147147

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

152152
// `if (x) foo()` -> `x && foo()`
@@ -221,7 +221,7 @@ impl<'a> PeepholeOptimizations {
221221
let consequent = Self::get_block_expression(&mut if_stmt.consequent, ctx);
222222
let else_branch = if_stmt.alternate.as_mut().unwrap();
223223
let alternate = Self::get_block_expression(else_branch, ctx);
224-
let expr = Self::minimize_conditional(
224+
let expr = self.minimize_conditional(
225225
if_stmt.span,
226226
test,
227227
consequent,
@@ -268,19 +268,21 @@ impl<'a> PeepholeOptimizations {
268268
}
269269

270270
pub fn minimize_conditional(
271+
&self,
271272
span: Span,
272273
test: Expression<'a>,
273274
consequent: Expression<'a>,
274275
alternate: Expression<'a>,
275276
ctx: Ctx<'a, '_>,
276277
) -> Expression<'a> {
277278
let mut cond_expr = ctx.ast.conditional_expression(span, test, consequent, alternate);
278-
Self::try_minimize_conditional(&mut cond_expr, ctx)
279+
self.try_minimize_conditional(&mut cond_expr, ctx)
279280
.unwrap_or_else(|| Expression::ConditionalExpression(ctx.ast.alloc(cond_expr)))
280281
}
281282

282283
// `MangleIfExpr`: <https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745>
283284
fn try_minimize_conditional(
285+
&self,
284286
expr: &mut ConditionalExpression<'a>,
285287
ctx: Ctx<'a, '_>,
286288
) -> Option<Expression<'a>> {
@@ -293,7 +295,7 @@ impl<'a> PeepholeOptimizations {
293295
let Expression::SequenceExpression(sequence_expr) = &mut sequence else {
294296
unreachable!()
295297
};
296-
let expr = Self::minimize_conditional(
298+
let expr = self.minimize_conditional(
297299
span,
298300
sequence_expr.expressions.pop().unwrap(),
299301
ctx.ast.move_expression(&mut expr.consequent),
@@ -310,9 +312,9 @@ impl<'a> PeepholeOptimizations {
310312
let test = ctx.ast.move_expression(&mut test_expr.argument);
311313
let consequent = ctx.ast.move_expression(&mut expr.alternate);
312314
let alternate = ctx.ast.move_expression(&mut expr.consequent);
313-
return Some(Self::minimize_conditional(
314-
expr.span, test, consequent, alternate, ctx,
315-
));
315+
return Some(
316+
self.minimize_conditional(expr.span, test, consequent, alternate, ctx),
317+
);
316318
}
317319
}
318320
Expression::Identifier(id) => {
@@ -351,9 +353,9 @@ impl<'a> PeepholeOptimizations {
351353
let test = ctx.ast.move_expression(&mut expr.test);
352354
let consequent = ctx.ast.move_expression(&mut expr.consequent);
353355
let alternate = ctx.ast.move_expression(&mut expr.alternate);
354-
return Some(Self::minimize_conditional(
355-
expr.span, test, alternate, consequent, ctx,
356-
));
356+
return Some(
357+
self.minimize_conditional(expr.span, test, alternate, consequent, ctx),
358+
);
357359
}
358360
}
359361
_ => {}
@@ -555,7 +557,7 @@ impl<'a> PeepholeOptimizations {
555557
let alternate_first_arg =
556558
ctx.ast.move_expression(alternate.arguments[0].to_expression_mut());
557559
let mut args = std::mem::replace(&mut consequent.arguments, ctx.ast.vec());
558-
let cond_expr = Self::minimize_conditional(
560+
let cond_expr = self.minimize_conditional(
559561
expr.test.span(),
560562
ctx.ast.move_expression(&mut expr.test),
561563
consequent_first_arg,
@@ -569,11 +571,52 @@ impl<'a> PeepholeOptimizations {
569571
}
570572

571573
// Not part of esbuild
572-
if let Some(e) = Self::try_merge_conditional_expression_inside(expr, ctx) {
574+
if let Some(e) = self.try_merge_conditional_expression_inside(expr, ctx) {
573575
return Some(e);
574576
}
575577

576-
// TODO: Try using the "??" or "?." operators
578+
// Try using the "??" or "?." operators
579+
if self.target >= ESTarget::ES2020 {
580+
if let Expression::BinaryExpression(test_binary) = &expr.test {
581+
if let Some(is_negate) = match test_binary.operator {
582+
BinaryOperator::Inequality => Some(true),
583+
BinaryOperator::Equality => Some(false),
584+
_ => None,
585+
} {
586+
// a == null / a != null
587+
if let Some((id_expr, ())) = Self::commutative_pair(
588+
(&test_binary.left, &test_binary.right),
589+
|a| {
590+
if let Expression::Identifier(id) = a {
591+
(!ctx.is_global_reference(id)).then_some(a)
592+
} else {
593+
None
594+
}
595+
},
596+
|b| b.is_null().then_some(()),
597+
) {
598+
// `a == null ? b : a` -> `a ?? b`
599+
// `a != null ? a : b` -> `a ?? b`
600+
let target_expr =
601+
if is_negate { &mut expr.consequent } else { &mut expr.alternate };
602+
if id_expr.content_eq(target_expr) {
603+
return Some(ctx.ast.expression_logical(
604+
expr.span,
605+
ctx.ast.move_expression(target_expr),
606+
LogicalOperator::Coalesce,
607+
ctx.ast.move_expression(if is_negate {
608+
&mut expr.alternate
609+
} else {
610+
&mut expr.consequent
611+
}),
612+
));
613+
}
614+
615+
// TODO: try using optional chaining
616+
}
617+
}
618+
}
619+
}
577620

578621
if ctx.expr_eq(&expr.alternate, &expr.consequent) {
579622
// TODO:
@@ -639,6 +682,7 @@ impl<'a> PeepholeOptimizations {
639682
///
640683
/// - `x ? a = 0 : a = 1` -> `a = x ? 0 : 1`
641684
fn try_merge_conditional_expression_inside(
685+
&self,
642686
expr: &mut ConditionalExpression<'a>,
643687
ctx: Ctx<'a, '_>,
644688
) -> Option<Expression<'a>> {
@@ -661,7 +705,7 @@ impl<'a> PeepholeOptimizations {
661705
{
662706
return None;
663707
}
664-
let cond_expr = Self::minimize_conditional(
708+
let cond_expr = self.minimize_conditional(
665709
expr.span,
666710
ctx.ast.move_expression(&mut expr.test),
667711
ctx.ast.move_expression(&mut consequent.right),
@@ -1151,6 +1195,14 @@ mod test {
11511195
};
11521196
use oxc_syntax::es_target::ESTarget;
11531197

1198+
fn test_es2019(source_text: &str, expected: &str) {
1199+
let target = ESTarget::ES2019;
1200+
assert_eq!(
1201+
run(source_text, Some(CompressOptions { target, ..CompressOptions::default() })),
1202+
run(expected, None)
1203+
);
1204+
}
1205+
11541206
/** Check that removing blocks with 1 child works */
11551207
#[test]
11561208
fn test_fold_one_child_blocks() {
@@ -2264,7 +2316,9 @@ mod test {
22642316
test("var a; a ? b(...c) : b(...e)", "var a; b(...a ? c : e)");
22652317
test("var a; a ? b(c) : b(e)", "var a; b(a ? c : e)");
22662318
test("a() != null ? a() : b", "a() == null ? b : a()");
2267-
// test("a != null ? a : b", "a ?? b");
2319+
test("var a; a != null ? a : b", "var a; a ?? b");
2320+
test("a != null ? a : b", "a == null ? b : a"); // accessing global `a` may have a getter with side effects
2321+
test_es2019("var a; a != null ? a : b", "var a; a == null ? b : a");
22682322
// test("a != null ? a.b.c[d](e) : undefined", "a?.b.c[d](e)");
22692323
test("cmp !== 0 ? cmp : (bar, cmp);", "cmp === 0 && bar, cmp;");
22702324
test("cmp === 0 ? cmp : (bar, cmp);", "cmp === 0 || bar, cmp;");

‎crates/oxc_minifier/src/peephole/minimize_statements.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -143,17 +143,12 @@ impl<'a> PeepholeOptimizations {
143143
{
144144
// "if (a, b) return c; return d;" => "return a, b ? c : d;"
145145
let test = sequence_expr.expressions.pop().unwrap();
146-
let mut b = Self::minimize_conditional(
147-
prev_if.span,
148-
test,
149-
left,
150-
right,
151-
ctx,
152-
);
146+
let mut b =
147+
self.minimize_conditional(prev_if.span, test, left, right, ctx);
153148
Self::join_sequence(&mut prev_if.test, &mut b, ctx)
154149
} else {
155150
// "if (a) return b; return c;" => "return a ? b : c;"
156-
let mut expr = Self::minimize_conditional(
151+
let mut expr = self.minimize_conditional(
157152
prev_if.span,
158153
ctx.ast.move_expression(&mut prev_if.test),
159154
left,

‎crates/oxc_minifier/tests/peephole/esbuild.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -592,12 +592,12 @@ fn js_parser_test() {
592592
test("a ? c : b || c", "a ? c : b || c;");
593593
test("a = b == null ? c : b", "a = b == null ? c : b;");
594594
test("a = b != null ? b : c", "a = b == null ? c : b;");
595-
// test("let b; a = b == null ? c : b", "let b;a = b ?? c;");
596-
// test("let b; a = b != null ? b : c", "let b;a = b ?? c;");
595+
test("let b; a = b == null ? c : b", "let b;a = b ?? c;");
596+
test("let b; a = b != null ? b : c", "let b;a = b ?? c;");
597597
test("let b; a = b == null ? b : c", "let b;a = b == null ? b : c;");
598-
// test("let b; a = b != null ? c : b", "let b;a = b != null ? c : b;");
599-
// test("let b; a = null == b ? c : b", "let b;a = b ?? c;");
600-
// test("let b; a = null != b ? b : c", "let b;a = b ?? c;");
598+
test("let b; a = b != null ? c : b", "let b;a = b == null ? b : c;");
599+
test("let b; a = null == b ? c : b", "let b;a = b ?? c;");
600+
test("let b; a = null != b ? b : c", "let b;a = b ?? c;");
601601
test("let b; a = null == b ? b : c", "let b;a = b == null ? b : c;");
602602
// test("let b; a = null != b ? c : b", "let b;a = b != null ? c : b;");
603603
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() {
608608
// test("let b; a = b !== null ? b : c", "let b;a = b !== null ? b : c;");
609609
test("let b; a = null === b ? c : b", "let b;a = b === null ? c : b;");
610610
// test("let b; a = null !== b ? b : c", "let b;a = b !== null ? b : c;");
611-
// test("let b; a = null === b || b === undefined ? c : b", "let b;a = b ?? c;");
612-
// test("let b; a = b !== undefined && b !== null ? b : c", "let b;a = b ?? c;");
611+
test("let b; a = null === b || b === undefined ? c : b", "let b;a = b ?? c;");
612+
test("let b; a = b !== undefined && b !== null ? b : c", "let b;a = b ?? c;");
613613
// test("a(b ? 0 : 0)", "a((b, 0));");
614614
// test("a(b ? +0 : -0)", "a(b ? 0 : -0);");
615615
// test("a(b ? +0 : 0)", "a((b, 0));");
@@ -639,10 +639,10 @@ fn js_parser_test() {
639639
test("return a ?? ((b ?? c) ?? (d ?? e))", "return a ?? b ?? c ?? d ?? e;");
640640
test("if (a) if (b) if (c) d", "a && b && c && d;");
641641
test("if (!a) if (!b) if (!c) d", "a || b || c || d;");
642-
// test(
643-
// "let a, b, c; return a != null ? a : b != null ? b : c",
644-
// "let a, b, c;return a ?? b ?? c;",
645-
// );
642+
test(
643+
"let a, b, c; return a != null ? a : b != null ? b : c",
644+
"let a, b, c;return a ?? b ?? c;",
645+
);
646646
test("if (a) return c; if (b) return d;", "if (a) return c;if (b) return d;");
647647
// test("if (a) return c; if (b) return c;", "if (a || b) return c;");
648648
}

‎tasks/minsize/minsize.snap

+10-10
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,25 @@ Original | minified | minified | gzip | gzip | Fixture
33
-------------------------------------------------------------------------------------
44
72.14 kB | 23.56 kB | 23.70 kB | 8.51 kB | 8.54 kB | react.development.js
55

6-
173.90 kB | 59.57 kB | 59.82 kB | 19.19 kB | 19.33 kB | moment.js
6+
173.90 kB | 59.55 kB | 59.82 kB | 19.19 kB | 19.33 kB | moment.js
77

8-
287.63 kB | 89.53 kB | 90.07 kB | 30.95 kB | 31.95 kB | jquery.js
8+
287.63 kB | 89.49 kB | 90.07 kB | 30.95 kB | 31.95 kB | jquery.js
99

10-
342.15 kB | 117.69 kB | 118.14 kB | 43.57 kB | 44.37 kB | vue.js
10+
342.15 kB | 117.68 kB | 118.14 kB | 43.56 kB | 44.37 kB | vue.js
1111

12-
544.10 kB | 71.44 kB | 72.48 kB | 25.87 kB | 26.20 kB | lodash.js
12+
544.10 kB | 71.43 kB | 72.48 kB | 25.87 kB | 26.20 kB | lodash.js
1313

14-
555.77 kB | 271.45 kB | 270.13 kB | 88.38 kB | 90.80 kB | d3.js
14+
555.77 kB | 271.25 kB | 270.13 kB | 88.35 kB | 90.80 kB | d3.js
1515

16-
1.01 MB | 441.00 kB | 458.89 kB | 122.51 kB | 126.71 kB | bundle.min.js
16+
1.01 MB | 440.96 kB | 458.89 kB | 122.50 kB | 126.71 kB | bundle.min.js
1717

1818
1.25 MB | 650.36 kB | 646.76 kB | 161.02 kB | 163.73 kB | three.js
1919

20-
2.14 MB | 718.80 kB | 724.14 kB | 162.18 kB | 181.07 kB | victory.js
20+
2.14 MB | 718.64 kB | 724.14 kB | 162.14 kB | 181.07 kB | victory.js
2121

22-
3.20 MB | 1.01 MB | 1.01 MB | 324.37 kB | 331.56 kB | echarts.js
22+
3.20 MB | 1.01 MB | 1.01 MB | 324.31 kB | 331.56 kB | echarts.js
2323

24-
6.69 MB | 2.30 MB | 2.31 MB | 469.25 kB | 488.28 kB | antd.js
24+
6.69 MB | 2.30 MB | 2.31 MB | 469.23 kB | 488.28 kB | antd.js
2525

26-
10.95 MB | 3.37 MB | 3.49 MB | 864.67 kB | 915.50 kB | typescript.js
26+
10.95 MB | 3.37 MB | 3.49 MB | 864.49 kB | 915.50 kB | typescript.js
2727

0 commit comments

Comments
 (0)