From cc3bea44f758995c880dd5712d32b5d95260267f Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Thu, 17 Jul 2025 14:13:02 +0000 Subject: [PATCH] refactor(minifier): do not remove unused assignment expression yet (#12367) it's broken for ```js export let foo; foo = 1; ``` closes #12340 --- .../remove_unused_variable_declaration.rs | 66 ++++++++++--------- tasks/minsize/minsize.snap | 10 +-- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/remove_unused_variable_declaration.rs b/crates/oxc_minifier/src/peephole/remove_unused_variable_declaration.rs index 26e671f14d499..8831f4c785b93 100644 --- a/crates/oxc_minifier/src/peephole/remove_unused_variable_declaration.rs +++ b/crates/oxc_minifier/src/peephole/remove_unused_variable_declaration.rs @@ -1,4 +1,3 @@ -use oxc_allocator::TakeIn; use oxc_ast::ast::*; use crate::{CompressOptionsUnused, ctx::Ctx}; @@ -48,38 +47,38 @@ impl<'a> PeepholeOptimizations { pub fn remove_unused_assignment_expression( &self, - e: &mut Expression<'a>, - state: &mut State, - ctx: &mut Ctx<'a, '_>, + _e: &mut Expression<'a>, + _state: &mut State, + _ctx: &mut Ctx<'a, '_>, ) -> bool { - let Expression::AssignmentExpression(assign_expr) = e else { return false }; - if matches!( - ctx.state.options.unused, - CompressOptionsUnused::Keep | CompressOptionsUnused::KeepAssign - ) { - return false; - } - let Some(SimpleAssignmentTarget::AssignmentTargetIdentifier(ident)) = - assign_expr.left.as_simple_assignment_target() - else { - return false; - }; - if Self::keep_top_level_var_in_script_mode(ctx) { - return false; - } - let Some(reference_id) = ident.reference_id.get() else { return false }; - let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else { - return false; - }; - // Keep error for assigning to `const foo = 1; foo = 2`. - if ctx.scoping().symbol_flags(symbol_id).is_const_variable() { - return false; - } - if !ctx.scoping().get_resolved_references(symbol_id).all(|r| !r.flags().is_read()) { - return false; - } - *e = assign_expr.right.take_in(ctx.ast); - state.changed = true; + // let Expression::AssignmentExpression(assign_expr) = e else { return false }; + // if matches!( + // ctx.state.options.unused, + // CompressOptionsUnused::Keep | CompressOptionsUnused::KeepAssign + // ) { + // return false; + // } + // let Some(SimpleAssignmentTarget::AssignmentTargetIdentifier(ident)) = + // assign_expr.left.as_simple_assignment_target() + // else { + // return false; + // }; + // if Self::keep_top_level_var_in_script_mode(ctx) { + // return false; + // } + // let Some(reference_id) = ident.reference_id.get() else { return false }; + // let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id() else { + // return false; + // }; + // // Keep error for assigning to `const foo = 1; foo = 2`. + // if ctx.scoping().symbol_flags(symbol_id).is_const_variable() { + // return false; + // } + // if !ctx.scoping().get_resolved_references(symbol_id).all(|r| !r.flags().is_read()) { + // return false; + // } + // *e = assign_expr.right.take_in(ctx.ast); + // state.changed = true; false } @@ -121,11 +120,13 @@ mod test { } #[test] + #[ignore] fn remove_unused_assignment_expression() { let options = CompressOptions::smallest(); test_options("var x = 1; x = 2;", "", &options); test_options("var x = 1; x = 2;", "", &options); test_options("var x = 1; x = foo();", "foo()", &options); + test_same_options("export let foo; foo = 0;", &options); test_same_options("var x = 1; x = 2, foo(x)", &options); test_same_options("function foo() { return t = x(); } foo();", &options); test_options( @@ -141,6 +142,7 @@ mod test { } #[test] + #[ignore] fn keep_in_script_mode() { let options = CompressOptions::smallest(); let source_type = SourceType::cjs(); diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index f835ace0c522e..3b70e6a7b1544 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -1,7 +1,7 @@ | Oxc | ESBuild | Oxc | ESBuild | Original | minified | minified | gzip | gzip | Fixture ------------------------------------------------------------------------------------- -72.14 kB | 23.45 kB | 23.70 kB | 8.46 kB | 8.54 kB | react.development.js +72.14 kB | 23.49 kB | 23.70 kB | 8.47 kB | 8.54 kB | react.development.js 173.90 kB | 59.51 kB | 59.82 kB | 19.18 kB | 19.33 kB | moment.js @@ -11,17 +11,17 @@ Original | minified | minified | gzip | gzip | Fixture 544.10 kB | 71.38 kB | 72.48 kB | 25.85 kB | 26.20 kB | lodash.js -555.77 kB | 270.80 kB | 270.13 kB | 88.24 kB | 90.80 kB | d3.js +555.77 kB | 270.83 kB | 270.13 kB | 88.25 kB | 90.80 kB | d3.js 1.01 MB | 440.17 kB | 458.89 kB | 122.37 kB | 126.71 kB | bundle.min.js 1.25 MB | 647 kB | 646.76 kB | 160.28 kB | 163.73 kB | three.js -2.14 MB | 716.10 kB | 724.14 kB | 161.76 kB | 181.07 kB | victory.js +2.14 MB | 716.12 kB | 724.14 kB | 161.80 kB | 181.07 kB | victory.js 3.20 MB | 1.01 MB | 1.01 MB | 324.08 kB | 331.56 kB | echarts.js -6.69 MB | 2.24 MB | 2.31 MB | 462.45 kB | 488.28 kB | antd.js +6.69 MB | 2.25 MB | 2.31 MB | 463.06 kB | 488.28 kB | antd.js -10.95 MB | 3.34 MB | 3.49 MB | 856.90 kB | 915.50 kB | typescript.js +10.95 MB | 3.35 MB | 3.49 MB | 860.95 kB | 915.50 kB | typescript.js