Skip to content

Commit

Permalink
fix(minifier): fix incorrect return value for (x ? true : y) (#8233)
Browse files Browse the repository at this point in the history
Need to check if the return value is used or not:

```
function foo() {
    return foo ? true : bar; // no transformed
}

foo ? true : bar; // transformed to `foo || bar;`
```
  • Loading branch information
Boshen committed Jan 3, 2025
1 parent e00e380 commit a698def
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 57 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,12 @@ oxc_mangler = { version = "0.44.0", path = "crates/oxc_mangler" }
oxc_minifier = { version = "0.44.0", path = "crates/oxc_minifier" }
oxc_napi = { version = "0.44.0", path = "crates/oxc_napi" }
oxc_parser = { version = "0.44.0", path = "crates/oxc_parser" }
oxc_parser_napi = { version = "0.44.0", path = "napi/parser" }
oxc_regular_expression = { version = "0.44.0", path = "crates/oxc_regular_expression" }
oxc_semantic = { version = "0.44.0", path = "crates/oxc_semantic" }
oxc_span = { version = "0.44.0", path = "crates/oxc_span" }
oxc_syntax = { version = "0.44.0", path = "crates/oxc_syntax" }
oxc_transform_napi = { version = "0.44.0", path = "napi/transform" }
oxc_parser_napi = { version = "0.44.0", path = "napi/parser" }
oxc_transformer = { version = "0.44.0", path = "crates/oxc_transformer" }
oxc_traverse = { version = "0.44.0", path = "crates/oxc_traverse" }

Expand Down Expand Up @@ -203,7 +203,7 @@ walkdir = "2.5.0"
wasm-bindgen = "0.2.99"

[workspace.metadata.cargo-shear]
ignored = ["napi", "oxc_transform_napi", "prettyplease"]
ignored = ["napi", "oxc_transform_napi", "oxc_parser_napi", "prettyplease"]

[profile.dev]
# Disabling debug info speeds up local and CI builds,
Expand Down
50 changes: 2 additions & 48 deletions crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,52 +383,6 @@ impl<'a> PeepholeMinimizeConditions {
}
}

// `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,
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,
ident,
LogicalOperator::And,
ctx.ast.move_expression(&mut expr.consequent),
));
}

None
}
}
Expand Down Expand Up @@ -732,8 +686,8 @@ mod test {
fn test_minimize_expr_condition() {
fold("(x ? true : false) && y()", "!!x && y()");
fold("(x ? false : true) && y()", "!x && y()");
fold("(x ? true : y) && y()", "(x || y) && y()");
fold("(x ? y : false) && y()", "(x && y) && y()");
// fold("(x ? true : y) && y()", "(x || y) && y()");
// fold("(x ? y : false) && y()", "(x && y) && y()");
fold("var x; (x && true) && y()", "var x; x && y()");
fold("var x; (x && false) && y()", "var x; false && y()");
fold("(x && true) && y()", "x && y()");
Expand Down
14 changes: 7 additions & 7 deletions tasks/minsize/minsize.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ Original | minified | minified | gzip | gzip | Fixture

287.63 kB | 90.16 kB | 90.07 kB | 32.08 kB | 31.95 kB | jquery.js

342.15 kB | 118.23 kB | 118.14 kB | 44.52 kB | 44.37 kB | vue.js
342.15 kB | 118.23 kB | 118.14 kB | 44.53 kB | 44.37 kB | vue.js

544.10 kB | 71.81 kB | 72.48 kB | 26.19 kB | 26.20 kB | lodash.js

555.77 kB | 273.19 kB | 270.13 kB | 90.99 kB | 90.80 kB | d3.js

1.01 MB | 460.32 kB | 458.89 kB | 126.84 kB | 126.71 kB | bundle.min.js
1.01 MB | 460.32 kB | 458.89 kB | 126.85 kB | 126.71 kB | bundle.min.js

1.25 MB | 652.73 kB | 646.76 kB | 163.55 kB | 163.73 kB | three.js
1.25 MB | 652.73 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js

2.14 MB | 726.33 kB | 724.14 kB | 180.21 kB | 181.07 kB | victory.js
2.14 MB | 726.33 kB | 724.14 kB | 180.20 kB | 181.07 kB | victory.js

3.20 MB | 1.01 MB | 1.01 MB | 331.91 kB | 331.56 kB | echarts.js
3.20 MB | 1.01 MB | 1.01 MB | 331.92 kB | 331.56 kB | echarts.js

6.69 MB | 2.32 MB | 2.31 MB | 492.81 kB | 488.28 kB | antd.js
6.69 MB | 2.32 MB | 2.31 MB | 492.82 kB | 488.28 kB | antd.js

10.95 MB | 3.50 MB | 3.49 MB | 909.52 kB | 915.50 kB | typescript.js
10.95 MB | 3.50 MB | 3.49 MB | 909.58 kB | 915.50 kB | typescript.js

0 comments on commit a698def

Please sign in to comment.