Skip to content

Commit f8a8c0c

Browse files
BoshenBoshen
committed
refactor(minifier): do not remove unused assignment expression yet (#12367)
it's broken for ```js export let foo; foo = 1; ``` closes #12340
1 parent ed696b5 commit f8a8c0c

File tree

2 files changed

+39
-37
lines changed

2 files changed

+39
-37
lines changed

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

Lines changed: 34 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use oxc_allocator::TakeIn;
21
use oxc_ast::ast::*;
32

43
use crate::{CompressOptionsUnused, ctx::Ctx};
@@ -48,38 +47,38 @@ impl<'a> PeepholeOptimizations {
4847

4948
pub fn remove_unused_assignment_expression(
5049
&self,
51-
e: &mut Expression<'a>,
52-
state: &mut State,
53-
ctx: &mut Ctx<'a, '_>,
50+
_e: &mut Expression<'a>,
51+
_state: &mut State,
52+
_ctx: &mut Ctx<'a, '_>,
5453
) -> bool {
55-
let Expression::AssignmentExpression(assign_expr) = e else { return false };
56-
if matches!(
57-
ctx.state.options.unused,
58-
CompressOptionsUnused::Keep | CompressOptionsUnused::KeepAssign
59-
) {
60-
return false;
61-
}
62-
let Some(SimpleAssignmentTarget::AssignmentTargetIdentifier(ident)) =
63-
assign_expr.left.as_simple_assignment_target()
64-
else {
65-
return false;
66-
};
67-
if Self::keep_top_level_var_in_script_mode(ctx) {
68-
return false;
69-
}
70-
let Some(reference_id) = ident.reference_id.get() else { return false };
71-
let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else {
72-
return false;
73-
};
74-
// Keep error for assigning to `const foo = 1; foo = 2`.
75-
if ctx.scoping().symbol_flags(symbol_id).is_const_variable() {
76-
return false;
77-
}
78-
if !ctx.scoping().get_resolved_references(symbol_id).all(|r| !r.flags().is_read()) {
79-
return false;
80-
}
81-
*e = assign_expr.right.take_in(ctx.ast);
82-
state.changed = true;
54+
// let Expression::AssignmentExpression(assign_expr) = e else { return false };
55+
// if matches!(
56+
// ctx.state.options.unused,
57+
// CompressOptionsUnused::Keep | CompressOptionsUnused::KeepAssign
58+
// ) {
59+
// return false;
60+
// }
61+
// let Some(SimpleAssignmentTarget::AssignmentTargetIdentifier(ident)) =
62+
// assign_expr.left.as_simple_assignment_target()
63+
// else {
64+
// return false;
65+
// };
66+
// if Self::keep_top_level_var_in_script_mode(ctx) {
67+
// return false;
68+
// }
69+
// let Some(reference_id) = ident.reference_id.get() else { return false };
70+
// let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else {
71+
// return false;
72+
// };
73+
// // Keep error for assigning to `const foo = 1; foo = 2`.
74+
// if ctx.scoping().symbol_flags(symbol_id).is_const_variable() {
75+
// return false;
76+
// }
77+
// if !ctx.scoping().get_resolved_references(symbol_id).all(|r| !r.flags().is_read()) {
78+
// return false;
79+
// }
80+
// *e = assign_expr.right.take_in(ctx.ast);
81+
// state.changed = true;
8382
false
8483
}
8584

@@ -121,11 +120,13 @@ mod test {
121120
}
122121

123122
#[test]
123+
#[ignore]
124124
fn remove_unused_assignment_expression() {
125125
let options = CompressOptions::smallest();
126126
test_options("var x = 1; x = 2;", "", &options);
127127
test_options("var x = 1; x = 2;", "", &options);
128128
test_options("var x = 1; x = foo();", "foo()", &options);
129+
test_same_options("export let foo; foo = 0;", &options);
129130
test_same_options("var x = 1; x = 2, foo(x)", &options);
130131
test_same_options("function foo() { return t = x(); } foo();", &options);
131132
test_options(
@@ -141,6 +142,7 @@ mod test {
141142
}
142143

143144
#[test]
145+
#[ignore]
144146
fn keep_in_script_mode() {
145147
let options = CompressOptions::smallest();
146148
let source_type = SourceType::cjs();

‎tasks/minsize/minsize.snap‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
| Oxc | ESBuild | Oxc | ESBuild |
22
Original | minified | minified | gzip | gzip | Fixture
33
-------------------------------------------------------------------------------------
4-
72.14 kB | 23.45 kB | 23.70 kB | 8.46 kB | 8.54 kB | react.development.js
4+
72.14 kB | 23.49 kB | 23.70 kB | 8.47 kB | 8.54 kB | react.development.js
55

66
173.90 kB | 59.51 kB | 59.82 kB | 19.18 kB | 19.33 kB | moment.js
77

@@ -11,17 +11,17 @@ Original | minified | minified | gzip | gzip | Fixture
1111

1212
544.10 kB | 71.38 kB | 72.48 kB | 25.85 kB | 26.20 kB | lodash.js
1313

14-
555.77 kB | 270.80 kB | 270.13 kB | 88.24 kB | 90.80 kB | d3.js
14+
555.77 kB | 270.83 kB | 270.13 kB | 88.25 kB | 90.80 kB | d3.js
1515

1616
1.01 MB | 440.17 kB | 458.89 kB | 122.37 kB | 126.71 kB | bundle.min.js
1717

1818
1.25 MB | 647 kB | 646.76 kB | 160.28 kB | 163.73 kB | three.js
1919

20-
2.14 MB | 716.10 kB | 724.14 kB | 161.76 kB | 181.07 kB | victory.js
20+
2.14 MB | 716.12 kB | 724.14 kB | 161.80 kB | 181.07 kB | victory.js
2121

2222
3.20 MB | 1.01 MB | 1.01 MB | 324.08 kB | 331.56 kB | echarts.js
2323

24-
6.69 MB | 2.24 MB | 2.31 MB | 462.45 kB | 488.28 kB | antd.js
24+
6.69 MB | 2.25 MB | 2.31 MB | 463.06 kB | 488.28 kB | antd.js
2525

26-
10.95 MB | 3.34 MB | 3.49 MB | 856.90 kB | 915.50 kB | typescript.js
26+
10.95 MB | 3.35 MB | 3.49 MB | 860.95 kB | 915.50 kB | typescript.js
2727

0 commit comments

Comments
 (0)