refactor(minifier): remove the buggy minimize_exit_points implement…

…ation (#8349)
oxc-project · Jan 9, 2025 · 09f0f48 · 09f0f48
1 parent 3ba0b53
commit 09f0f48
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 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
@@ -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
@@ -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
@@ -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