Skip to content

Commit 6585463

Browse files
committed
fix(minifier): always keep the last value of sequence expression (#8490)
Caught in react app: ``` React.useEffect(() => { isMountRef.current = false; return () => { isMountRef.current = true; }; }, []); ``` -> ``` React.useEffect(() => isMountRef.current = !1, []); ``` Two problems: there were no unit tests guarding this, no good way of knowing when code gets deleted.
1 parent 3174675 commit 6585463

1 file changed

Lines changed: 3 additions & 6 deletions

File tree

crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -496,8 +496,6 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
496496
sequence_expr: &mut SequenceExpression<'a>,
497497
ctx: Ctx<'a, 'b>,
498498
) -> Option<Expression<'a>> {
499-
let should_include_ret_val =
500-
!matches!(ctx.parent(), Ancestor::ExpressionStatementExpression(_));
501499
let should_keep_as_sequence_expr = matches!(
502500
ctx.parent(),
503501
Ancestor::CallExpressionCallee(_) | Ancestor::TaggedTemplateExpressionTag(_)
@@ -510,9 +508,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
510508
let (should_fold, new_len) = sequence_expr.expressions.iter().enumerate().fold(
511509
(false, 0),
512510
|(mut should_fold, mut new_len), (i, expr)| {
513-
if expr.may_have_side_effects()
514-
|| (should_include_ret_val && i == sequence_expr.expressions.len() - 1)
515-
{
511+
if i == sequence_expr.expressions.len() - 1 || expr.may_have_side_effects() {
516512
new_len += 1;
517513
} else {
518514
should_fold = true;
@@ -529,7 +525,7 @@ impl<'a, 'b> PeepholeRemoveDeadCode {
529525
let mut new_exprs = ctx.ast.vec_with_capacity(new_len);
530526
let len = sequence_expr.expressions.len();
531527
for (i, expr) in sequence_expr.expressions.iter_mut().enumerate() {
532-
if expr.may_have_side_effects() || (should_include_ret_val && i == len - 1) {
528+
if i == len - 1 || expr.may_have_side_effects() {
533529
new_exprs.push(ctx.ast.move_expression(expr));
534530
}
535531
}
@@ -744,6 +740,7 @@ mod test {
744740
fold("var obj = Object((null, 2, 3), 1, 2);", "var obj = Object(3, 1, 2);");
745741
fold_same("(0 instanceof 0, foo)");
746742
fold_same("(0 in 0, foo)");
743+
fold_same("React.useEffect(() => (isMountRef.current = false, () => { isMountRef.current = true; }), [])");
747744
}
748745

749746
#[test]

0 commit comments

Comments
 (0)