From 695fe58a7994e54f8610a800e911073f1fb275c0 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Thu, 9 Jan 2025 02:47:08 +0000 Subject: [PATCH] refactor(minifier): remove the buggy `minimize_exit_points` implementation (#8349) --- .../src/ast_passes/minimize_exit_points.rs | 98 +------------------ crates/oxc_minifier/tests/ast_passes/mod.rs | 13 ++- tasks/minsize/minsize.snap | 24 ++--- 3 files changed, 23 insertions(+), 112 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/minimize_exit_points.rs b/crates/oxc_minifier/src/ast_passes/minimize_exit_points.rs index 600587a31c0111..38f375f717243a 100644 --- a/crates/oxc_minifier/src/ast_passes/minimize_exit_points.rs +++ b/crates/oxc_minifier/src/ast_passes/minimize_exit_points.rs @@ -1,8 +1,5 @@ -use oxc_allocator::Vec; use oxc_ast::ast::*; -use oxc_semantic::ScopeFlags; -use oxc_span::{GetSpan, SPAN}; -use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse, TraverseCtx}; +use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse}; use crate::CompressorPass; @@ -21,68 +18,12 @@ impl<'a> CompressorPass<'a> for MinimizeExitPoints { } } -impl<'a> Traverse<'a> for MinimizeExitPoints { - fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, _ctx: &mut TraverseCtx<'a>) { - self.remove_last_return(&mut body.statements); - } - - fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { - self.fold_if_return(stmts, ctx); - } -} +impl Traverse<'_> for MinimizeExitPoints {} -impl<'a> MinimizeExitPoints { +impl MinimizeExitPoints { pub fn new() -> Self { Self { changed: false } } - - // `function foo() { return }` -> `function foo() {}` - fn remove_last_return(&mut self, stmts: &mut Vec<'a, Statement<'a>>) { - if let Some(last) = stmts.last() { - if matches!(last, Statement::ReturnStatement(ret) if ret.argument.is_none()) { - stmts.pop(); - self.changed = true; - } - } - } - - // `if(x)return;foo` -> `if(!x)foo;` - fn fold_if_return(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { - if stmts.len() <= 1 { - return; - } - let Some(index) = stmts.iter().position(|stmt| { - if let Statement::IfStatement(if_stmt) = stmt { - if if_stmt.alternate.is_none() - && matches!( - if_stmt.consequent.get_one_child(), - Some(Statement::ReturnStatement(s)) if s.argument.is_none() - ) - { - return true; - } - } - false - }) else { - return; - }; - let Some(stmts_rest) = stmts.get_mut(index + 1..) else { return }; - let body = ctx.ast.vec_from_iter(stmts_rest.iter_mut().map(|s| ctx.ast.move_statement(s))); - let Statement::IfStatement(if_stmt) = &mut stmts[index] else { unreachable!() }; - let scope_id = ctx.create_child_scope_of_current(ScopeFlags::empty()); - if_stmt.test = match ctx.ast.move_expression(&mut if_stmt.test) { - Expression::UnaryExpression(unary_expr) if unary_expr.operator.is_not() => { - unary_expr.unbox().argument - } - e => ctx.ast.expression_unary(e.span(), UnaryOperator::LogicalNot, e), - }; - if_stmt.alternate = None; - if_stmt.consequent = Statement::BlockStatement( - ctx.ast.alloc_block_statement_with_scope_id(SPAN, body, scope_id), - ); - stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_))); - self.changed = true; - } } #[cfg(test)] @@ -101,39 +42,6 @@ mod test { fold(source_text, source_text); } - // oxc - - #[test] - fn simple() { - fold( - "function foo() { if (foo) return; bar; quaz; }", - "function foo() { if (!foo) { bar; quaz; } }", - ); - fold( - "function foo() { if (!foo) return; bar; quaz; }", - "function foo() { if (foo) { bar; quaz; } }", - ); - fold( - "function foo() { x; if (foo) return; bar; quaz; }", - "function foo() { x; if (!foo) { bar; quaz; } }", - ); - fold( - "function foo() { x; if (!foo) return; bar; quaz; }", - "function foo() { x; if (foo) { bar; quaz; } }", - ); - fold_same("function foo() { if (foo) return }"); - fold_same("function foo() { if (foo) return bar; baz }"); - } - - #[test] - fn remove_last_return() { - fold("function () {return}", "function () {}"); - fold("function () {a;b;return}", "function () {a;b;}"); - fold_same("function () { if(foo) { return } }"); - } - - // closure - #[test] #[ignore] fn test_break_optimization() { diff --git a/crates/oxc_minifier/tests/ast_passes/mod.rs b/crates/oxc_minifier/tests/ast_passes/mod.rs index 13c8ad06aa97ad..e86d2f04a4478f 100644 --- a/crates/oxc_minifier/tests/ast_passes/mod.rs +++ b/crates/oxc_minifier/tests/ast_passes/mod.rs @@ -44,13 +44,16 @@ fn cjs() { });"#, " Object.keys(_index6).forEach(function(key) { - !(key === 'default' || key === '__esModule') && !Object.prototype.hasOwnProperty.call(_exportNames, key) && (key in exports && exports[key] === _index6[key] || Object.defineProperty(exports, key, { - enumerable: !0, + if (key === 'default' || key === '__esModule') return; + if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return; + if (key in exports && exports[key] === _index6[key]) return; + Object.defineProperty(exports, key, { + enumerable: true, get: function() { - return _index6[key]; + return _index6[key]; } - })); - });" + }); +});", ); } diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index c4958f8ba25d27..c3a775666c230f 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -1,27 +1,27 @@ | Oxc | ESBuild | Oxc | ESBuild | Original | minified | minified | gzip | gzip | Fixture ------------------------------------------------------------------------------------- -72.14 kB | 23.68 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js +72.14 kB | 23.71 kB | 23.70 kB | 8.62 kB | 8.54 kB | react.development.js -173.90 kB | 59.77 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js +173.90 kB | 59.80 kB | 59.82 kB | 19.41 kB | 19.33 kB | moment.js -287.63 kB | 90.02 kB | 90.07 kB | 32.05 kB | 31.95 kB | jquery.js +287.63 kB | 90.14 kB | 90.07 kB | 32.07 kB | 31.95 kB | jquery.js -342.15 kB | 118.12 kB | 118.14 kB | 44.51 kB | 44.37 kB | vue.js +342.15 kB | 118.38 kB | 118.14 kB | 44.53 kB | 44.37 kB | vue.js -544.10 kB | 71.72 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js +544.10 kB | 71.75 kB | 72.48 kB | 26.16 kB | 26.20 kB | lodash.js -555.77 kB | 272.81 kB | 270.13 kB | 90.92 kB | 90.80 kB | d3.js +555.77 kB | 273.20 kB | 270.13 kB | 90.93 kB | 90.80 kB | d3.js -1.01 MB | 460.21 kB | 458.89 kB | 126.83 kB | 126.71 kB | bundle.min.js +1.01 MB | 460.62 kB | 458.89 kB | 126.89 kB | 126.71 kB | bundle.min.js -1.25 MB | 652.53 kB | 646.76 kB | 163.51 kB | 163.73 kB | three.js +1.25 MB | 653.02 kB | 646.76 kB | 163.55 kB | 163.73 kB | three.js -2.14 MB | 726 kB | 724.14 kB | 180.13 kB | 181.07 kB | victory.js +2.14 MB | 726.50 kB | 724.14 kB | 180.19 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 331.78 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 331.98 kB | 331.56 kB | echarts.js -6.69 MB | 2.32 MB | 2.31 MB | 492.61 kB | 488.28 kB | antd.js +6.69 MB | 2.32 MB | 2.31 MB | 492.75 kB | 488.28 kB | antd.js -10.95 MB | 3.50 MB | 3.49 MB | 908.24 kB | 915.50 kB | typescript.js +10.95 MB | 3.50 MB | 3.49 MB | 909.08 kB | 915.50 kB | typescript.js