From 94d3c319337e674c8918943a5227c2efb7895900 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 7 Aug 2024 05:09:32 +0000 Subject: [PATCH] fix(minifier): avoid removing function declaration from `KeepVar` (#4722) --- crates/oxc_ast_macros/Cargo.toml | 2 +- .../src/ast_passes/remove_dead_code.rs | 17 ++++++- crates/oxc_minifier/src/keep_var.rs | 49 +++++++++++++------ .../tests/oxc/remove_dead_code.rs | 28 ++++++++--- tasks/minsize/minsize.snap | 18 +++---- 5 files changed, 79 insertions(+), 35 deletions(-) diff --git a/crates/oxc_ast_macros/Cargo.toml b/crates/oxc_ast_macros/Cargo.toml index c73f75d991386..f52b3565b3496 100644 --- a/crates/oxc_ast_macros/Cargo.toml +++ b/crates/oxc_ast_macros/Cargo.toml @@ -22,5 +22,5 @@ doctest = false [dependencies] quote = { workspace = true } -syn = { workspace = true, features = ["full", "parsing", "proc-macro", "printing"] } +syn = { workspace = true, features = ["full", "parsing", "printing", "proc-macro"] } proc-macro2 = { workspace = true } diff --git a/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs index 4e1b97784effc..b700bbaf2d224 100644 --- a/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/remove_dead_code.rs @@ -55,6 +55,9 @@ impl<'a> RemoveDeadCode<'a> { } let Some(index) = index else { return }; + if index == stmts.len() - 1 { + return; + } let mut keep_var = KeepVar::new(self.ast); @@ -62,7 +65,19 @@ impl<'a> RemoveDeadCode<'a> { keep_var.visit_statement(stmt); } - stmts.drain(index + 1..); + let mut i = 0; + stmts.retain(|s| { + i += 1; + if i - 1 <= index { + return true; + } + // keep function declaration + if matches!(s.as_declaration(), Some(Declaration::FunctionDeclaration(_))) { + return true; + } + false + }); + if let Some(stmt) = keep_var.get_variable_declaration_statement() { stmts.push(stmt); } diff --git a/crates/oxc_minifier/src/keep_var.rs b/crates/oxc_minifier/src/keep_var.rs index 7f698345ec3b9..90b9361f51c95 100644 --- a/crates/oxc_minifier/src/keep_var.rs +++ b/crates/oxc_minifier/src/keep_var.rs @@ -1,7 +1,6 @@ #[allow(clippy::wildcard_imports)] use oxc_ast::{ast::*, syntax_directed_operations::BoundNames, AstBuilder, Visit}; use oxc_span::{Atom, Span, SPAN}; -use oxc_syntax::scope::ScopeFlags; pub struct KeepVar<'a> { ast: AstBuilder<'a>, @@ -9,23 +8,41 @@ pub struct KeepVar<'a> { } impl<'a> Visit<'a> for KeepVar<'a> { - fn visit_variable_declaration(&mut self, decl: &VariableDeclaration<'a>) { - if decl.kind.is_var() { - decl.bound_names(&mut |ident| { - self.vars.push((ident.name.clone(), ident.span)); - }); + fn visit_statement(&mut self, it: &Statement<'a>) { + // Only visit blocks where vars could be hoisted + match it { + Statement::BlockStatement(it) => self.visit_block_statement(it), + Statement::BreakStatement(it) => self.visit_break_statement(it), + Statement::ContinueStatement(it) => self.visit_continue_statement(it), + // Statement::DebuggerStatement(it) => self.visit_debugger_statement(it), + Statement::DoWhileStatement(it) => self.visit_do_while_statement(it), + // Statement::EmptyStatement(it) => self.visit_empty_statement(it), + // Statement::ExpressionStatement(it) => self.visit_expression_statement(it), + Statement::ForInStatement(it) => self.visit_for_in_statement(it), + Statement::ForOfStatement(it) => self.visit_for_of_statement(it), + Statement::ForStatement(it) => self.visit_for_statement(it), + Statement::IfStatement(it) => self.visit_if_statement(it), + Statement::LabeledStatement(it) => self.visit_labeled_statement(it), + // Statement::ReturnStatement(it) => self.visit_return_statement(it), + Statement::SwitchStatement(it) => self.visit_switch_statement(it), + // Statement::ThrowStatement(it) => self.visit_throw_statement(it), + Statement::TryStatement(it) => self.visit_try_statement(it), + Statement::WhileStatement(it) => self.visit_while_statement(it), + Statement::WithStatement(it) => self.visit_with_statement(it), + // match_declaration!(Statement) => visitor.visit_declaration(it.to_declaration()), + // match_module_declaration!(Statement) => { + // visitor.visit_module_declaration(it.to_module_declaration()) + // } + Statement::VariableDeclaration(decl) => { + if decl.kind.is_var() { + decl.bound_names(&mut |ident| { + self.vars.push((ident.name.clone(), ident.span)); + }); + } + } + _ => {} } } - - fn visit_function(&mut self, _it: &Function<'a>, _flags: ScopeFlags) { - /* skip functions */ - } - - fn visit_arrow_function_expression(&mut self, _it: &ArrowFunctionExpression<'a>) {} - - fn visit_class(&mut self, _it: &Class<'a>) { - /* skip classes */ - } } impl<'a> KeepVar<'a> { diff --git a/crates/oxc_minifier/tests/oxc/remove_dead_code.rs b/crates/oxc_minifier/tests/oxc/remove_dead_code.rs index efd59c59170af..a847a7200a9af 100644 --- a/crates/oxc_minifier/tests/oxc/remove_dead_code.rs +++ b/crates/oxc_minifier/tests/oxc/remove_dead_code.rs @@ -24,12 +24,6 @@ fn test(source_text: &str, expected: &str) { assert_eq!(minified, expected, "for source {source_text}"); } -fn test_same(source_text: &str) { - let minified = print(source_text, true); - let expected = print(source_text, false); - assert_eq!(minified, expected, "for source {source_text}"); -} - #[test] fn dce_if_statement() { test("if (true) { foo }", "{ foo }"); @@ -127,18 +121,36 @@ fn dce_logical_expression() { #[test] fn dce_var_hoisting() { - test_same( + test( + "function f() { + return () => { + var x; + } + REMOVE; + function KEEP() {} + REMOVE; + }", "function f() { return () => { var x; } + function KEEP() {} }", ); - test_same( + test( + "function f() { + return function g() { + var x; + } + REMOVE; + function KEEP() {} + REMOVE; + }", "function f() { return function g() { var x; } + function KEEP() {} }", ); } diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index c4e8339c8b3ac..c554d778334ea 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -2,25 +2,25 @@ Original | Minified | esbuild | Gzip | esbuild 72.14 kB | 24.32 kB | 23.70 kB | 8.72 kB | 8.54 kB | react.development.js -173.90 kB | 61.80 kB | 59.82 kB | 19.57 kB | 19.33 kB | moment.js +173.90 kB | 61.80 kB | 59.82 kB | 19.58 kB | 19.33 kB | moment.js 287.63 kB | 92.91 kB | 90.07 kB | 32.33 kB | 31.95 kB | jquery.js -342.15 kB | 122.97 kB | 118.14 kB | 45.08 kB | 44.37 kB | vue.js +342.15 kB | 122.97 kB | 118.14 kB | 45.05 kB | 44.37 kB | vue.js -544.10 kB | 74.71 kB | 72.48 kB | 26.24 kB | 26.20 kB | lodash.js +544.10 kB | 74.71 kB | 72.48 kB | 26.22 kB | 26.20 kB | lodash.js -555.77 kB | 274.92 kB | 270.13 kB | 91.50 kB | 90.80 kB | d3.js +555.77 kB | 274.82 kB | 270.13 kB | 91.39 kB | 90.80 kB | d3.js -1.01 MB | 471.72 kB | 458.89 kB | 127.60 kB | 126.71 kB | bundle.min.js +1.01 MB | 471.74 kB | 458.89 kB | 127.57 kB | 126.71 kB | bundle.min.js -1.25 MB | 673.73 kB | 646.76 kB | 166.77 kB | 163.73 kB | three.js +1.25 MB | 673.75 kB | 646.76 kB | 166.76 kB | 163.73 kB | three.js -2.14 MB | 743.50 kB | 724.14 kB | 182.06 kB | 181.07 kB | victory.js +2.14 MB | 743.40 kB | 724.14 kB | 181.95 kB | 181.07 kB | victory.js -3.20 MB | 1.03 MB | 1.01 MB | 332.77 kB | 331.56 kB | echarts.js +3.20 MB | 1.03 MB | 1.01 MB | 332.46 kB | 331.56 kB | echarts.js 6.69 MB | 2.42 MB | 2.31 MB | 503.31 kB | 488.28 kB | antd.js -10.95 MB | 3.57 MB | 3.49 MB | 912.65 kB | 915.50 kB | typescript.js +10.95 MB | 3.57 MB | 3.49 MB | 912.42 kB | 915.50 kB | typescript.js