From e88cb559ee2fc6ff8475b51890171f042bfd4b77 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Sat, 28 Dec 2024 15:17:56 +0900 Subject: [PATCH 1/2] feat(minifier): remove unnecessary "use strict" --- .../src/ast_passes/peephole_fold_constants.rs | 9 +- .../src/ast_passes/remove_syntax.rs | 90 +++++++++++++++++++ crates/oxc_minifier/src/tester.rs | 9 +- tasks/minsize/minsize.snap | 6 +- tasks/minsize/src/lib.rs | 2 +- 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs b/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs index d0e55587a4aa3..32e70cfcd5194 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_fold_constants.rs @@ -527,7 +527,14 @@ mod test { fn test_nospace(source_text: &str, expected: &str) { let allocator = Allocator::default(); let mut pass = super::PeepholeFoldConstants::new(); - tester::test_impl(&allocator, source_text, expected, &mut pass, true); + tester::test_impl( + &allocator, + source_text, + expected, + &mut pass, + oxc_span::SourceType::mjs(), + true, + ); } fn test_same(source_text: &str) { diff --git a/crates/oxc_minifier/src/ast_passes/remove_syntax.rs b/crates/oxc_minifier/src/ast_passes/remove_syntax.rs index c72189134a13d..6682fad9347a7 100644 --- a/crates/oxc_minifier/src/ast_passes/remove_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/remove_syntax.rs @@ -21,6 +21,18 @@ impl<'a> CompressorPass<'a> for RemoveSyntax { } impl<'a> Traverse<'a> for RemoveSyntax { + fn enter_program(&mut self, program: &mut Program<'a>, ctx: &mut TraverseCtx<'a>) { + Self::drop_use_strict_directives_in_program(program, ctx); + } + + fn enter_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { + Self::drop_use_strict_directives_in_function_body(body, ctx); + } + + fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { + Self::drop_use_strict_directives_if_function_is_empty(body, ctx); + } + fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) { stmts.retain(|stmt| { !(matches!(stmt, Statement::EmptyStatement(_)) @@ -89,6 +101,38 @@ impl<'a> RemoveSyntax { let Some(ident) = obj.get_identifier_reference() else { return false }; ident.name == "console" } + + /// Drop `"use strict";` directives if the input is strict mode (e.g. written in ESM). + fn drop_use_strict_directives_in_program( + program: &mut Program<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { + if program.source_type.is_strict() { + program.directives.retain(|directive| !directive.is_use_strict()); + } + } + + /// Drop `"use strict";` directives if the parent scope is already strict mode. + fn drop_use_strict_directives_in_function_body( + body: &mut FunctionBody<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + let current_scope_id = ctx.current_scope_id(); + let Some(parent_scope_id) = ctx.scopes().get_parent_id(current_scope_id) else { return }; + if ctx.scopes().get_flags(parent_scope_id).is_strict_mode() { + body.directives.retain(|directive| !directive.is_use_strict()); + } + } + + /// Drop `"use strict";` directives if the function is empty. + fn drop_use_strict_directives_if_function_is_empty( + body: &mut FunctionBody<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { + if body.statements.is_empty() { + body.directives.retain(|directive| !directive.is_use_strict()); + } + } } #[cfg(test)] @@ -103,6 +147,23 @@ mod test { tester::test(&allocator, source_text, expected, &mut pass); } + fn test_script(source_text: &str, expected: &str) { + let allocator = Allocator::default(); + let mut pass = super::RemoveSyntax::new(CompressOptions::all_true()); + tester::test_impl( + &allocator, + source_text, + expected, + &mut pass, + oxc_span::SourceType::cjs(), + true, + ); + } + + fn test_script_same(source_text: &str) { + test_script(source_text, source_text); + } + #[test] fn parens() { test("(((x)))", "x"); @@ -118,4 +179,33 @@ mod test { fn drop_debugger() { test("debugger", ""); } + + #[test] + fn use_strict() { + test("'use strict';", ""); + + test_script( + "'use strict'; function foo() { 'use strict'; alert(1); }", + "'use strict'; function foo() { alert(1); }", + ); + test_script( + "'use strict'; const foo = () => { 'use strict'; alert(1); }", + "'use strict'; const foo = () => { alert(1); }", + ); + test_script_same("function foo() { 'use strict'; alert(1); }"); + test_script( + "function foo() { 'use strict'; return function foo() { 'use strict'; alert(1); }; } ", + "function foo() { 'use strict'; return function foo() { alert(1); }; } ", + ); + test_script( + "class Foo { foo() { 'use strict'; alert(1); } } ", + "class Foo { foo() { alert(1); } } ", + ); + test_script( + "const Foo = class { foo() { 'use strict'; alert(1); } } ", + "const Foo = class { foo() { alert(1); } } ", + ); + + test_script("function foo() { 'use strict';}", "function foo() {}"); + } } diff --git a/crates/oxc_minifier/src/tester.rs b/crates/oxc_minifier/src/tester.rs index 25e438531028a..938af1da6bda9 100644 --- a/crates/oxc_minifier/src/tester.rs +++ b/crates/oxc_minifier/src/tester.rs @@ -16,7 +16,7 @@ pub fn test<'a, P: CompressorPass<'a>>( expected: &'a str, pass: &mut P, ) { - test_impl(allocator, source_text, expected, pass, false); + test_impl(allocator, source_text, expected, pass, SourceType::mjs(), false); } pub fn test_impl<'a, P: CompressorPass<'a>>( @@ -24,10 +24,11 @@ pub fn test_impl<'a, P: CompressorPass<'a>>( source_text: &'a str, expected: &'a str, pass: &mut P, + source_type: SourceType, remove_whitespace: bool, ) { - let result = run(allocator, source_text, Some(pass), remove_whitespace); - let expected = run::

(allocator, expected, None, remove_whitespace); + let result = run(allocator, source_text, Some(pass), source_type, remove_whitespace); + let expected = run::

(allocator, expected, None, source_type, remove_whitespace); assert_eq!(result, expected, "\nfor source\n{source_text}\nexpect\n{expected}\ngot\n{result}"); } @@ -35,9 +36,9 @@ fn run<'a, P: CompressorPass<'a>>( allocator: &'a Allocator, source_text: &'a str, pass: Option<&mut P>, + source_type: SourceType, remove_whitespace: bool, ) -> String { - let source_type = SourceType::mjs(); let mut program = Parser::new(allocator, source_text, source_type).parse().program; if let Some(pass) = pass { diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 0d863febd2b80..068573b03eee7 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.74 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js +72.14 kB | 23.72 kB | 23.70 kB | 8.61 kB | 8.54 kB | react.development.js 173.90 kB | 60.22 kB | 59.82 kB | 19.49 kB | 19.33 kB | moment.js @@ -17,11 +17,11 @@ Original | minified | minified | gzip | gzip | Fixture 1.25 MB | 656.81 kB | 646.76 kB | 164.16 kB | 163.73 kB | three.js -2.14 MB | 735.33 kB | 724.14 kB | 180.99 kB | 181.07 kB | victory.js +2.14 MB | 735.31 kB | 724.14 kB | 180.98 kB | 181.07 kB | victory.js 3.20 MB | 1.01 MB | 1.01 MB | 332.27 kB | 331.56 kB | echarts.js -6.69 MB | 2.36 MB | 2.31 MB | 495.04 kB | 488.28 kB | antd.js +6.69 MB | 2.36 MB | 2.31 MB | 494.94 kB | 488.28 kB | antd.js 10.95 MB | 3.51 MB | 3.49 MB | 910.93 kB | 915.50 kB | typescript.js diff --git a/tasks/minsize/src/lib.rs b/tasks/minsize/src/lib.rs index 3c149505e568d..a384cfdec542f 100644 --- a/tasks/minsize/src/lib.rs +++ b/tasks/minsize/src/lib.rs @@ -124,7 +124,7 @@ pub fn run() -> Result<(), io::Error> { } fn minify_twice(file: &TestFile) -> String { - let source_type = SourceType::from_path(&file.file_name).unwrap(); + let source_type = SourceType::from_path(&file.file_name).unwrap().with_script(true); let options = MinifierOptions { mangle: Some(MangleOptions::default()), compress: CompressOptions::default(), From 4c0a41b1533d8469ebc3648748f18958af9c7679 Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Sat, 28 Dec 2024 16:18:31 +0900 Subject: [PATCH 2/2] fix(minifier): move drop_use_strict_directives_if_function_is_empty to PeepholeRemoveDeadCode --- crates/oxc_minifier/src/ast_passes/mod.rs | 9 +++++++++ .../ast_passes/peephole_remove_dead_code.rs | 19 +++++++++++++++++++ .../src/ast_passes/remove_syntax.rs | 16 ---------------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/crates/oxc_minifier/src/ast_passes/mod.rs b/crates/oxc_minifier/src/ast_passes/mod.rs index 459265a14fe03..99dc063b5f8d6 100644 --- a/crates/oxc_minifier/src/ast_passes/mod.rs +++ b/crates/oxc_minifier/src/ast_passes/mod.rs @@ -146,6 +146,7 @@ impl<'a> Traverse<'a> for LatePeepholeOptimizations { fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { self.x0_statement_fusion.exit_function_body(body, ctx); + self.x2_peephole_remove_dead_code.exit_function_body(body, ctx); } fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { @@ -232,6 +233,10 @@ impl<'a> Traverse<'a> for PeepholeOptimizations { self.x5_peephole_remove_dead_code.exit_program(program, ctx); } + fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { + self.x5_peephole_remove_dead_code.exit_function_body(body, ctx); + } + fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { self.x2_peephole_minimize_conditions.exit_statements(stmts, ctx); self.x5_peephole_remove_dead_code.exit_statements(stmts, ctx); @@ -300,6 +305,10 @@ impl<'a> Traverse<'a> for DeadCodeElimination { self.x2_peephole_remove_dead_code.exit_program(program, ctx); } + fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { + self.x2_peephole_remove_dead_code.exit_function_body(body, ctx); + } + fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { self.x2_peephole_remove_dead_code.exit_statements(stmts, ctx); } diff --git a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs index 18d23ed4b9353..0c66c0746cbda 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_remove_dead_code.rs @@ -24,6 +24,10 @@ impl<'a> CompressorPass<'a> for PeepholeRemoveDeadCode { } impl<'a> Traverse<'a> for PeepholeRemoveDeadCode { + fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { + Self::drop_use_strict_directives_if_function_is_empty(body, ctx); + } + fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { let ctx = Ctx(ctx); if let Some(new_stmt) = match stmt { @@ -124,6 +128,16 @@ impl<'a, 'b> PeepholeRemoveDeadCode { } } + /// Drop `"use strict";` directives if the function is empty. + fn drop_use_strict_directives_if_function_is_empty( + body: &mut FunctionBody<'a>, + _ctx: &mut TraverseCtx<'a>, + ) { + if body.statements.is_empty() { + body.directives.retain(|directive| !directive.is_use_strict()); + } + } + /// Remove block from single line blocks /// `{ block } -> block` fn try_optimize_block( @@ -436,6 +450,11 @@ mod test { test(js, expected); } + #[test] + fn use_strict() { + test("function foo() { 'use strict';}", "function foo() {}"); + } + #[test] fn test_fold_block() { fold("{{foo()}}", "foo()"); diff --git a/crates/oxc_minifier/src/ast_passes/remove_syntax.rs b/crates/oxc_minifier/src/ast_passes/remove_syntax.rs index 6682fad9347a7..b91d9d6b6beb4 100644 --- a/crates/oxc_minifier/src/ast_passes/remove_syntax.rs +++ b/crates/oxc_minifier/src/ast_passes/remove_syntax.rs @@ -29,10 +29,6 @@ impl<'a> Traverse<'a> for RemoveSyntax { Self::drop_use_strict_directives_in_function_body(body, ctx); } - fn exit_function_body(&mut self, body: &mut FunctionBody<'a>, ctx: &mut TraverseCtx<'a>) { - Self::drop_use_strict_directives_if_function_is_empty(body, ctx); - } - fn exit_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, _ctx: &mut TraverseCtx<'a>) { stmts.retain(|stmt| { !(matches!(stmt, Statement::EmptyStatement(_)) @@ -123,16 +119,6 @@ impl<'a> RemoveSyntax { body.directives.retain(|directive| !directive.is_use_strict()); } } - - /// Drop `"use strict";` directives if the function is empty. - fn drop_use_strict_directives_if_function_is_empty( - body: &mut FunctionBody<'a>, - _ctx: &mut TraverseCtx<'a>, - ) { - if body.statements.is_empty() { - body.directives.retain(|directive| !directive.is_use_strict()); - } - } } #[cfg(test)] @@ -205,7 +191,5 @@ mod test { "const Foo = class { foo() { 'use strict'; alert(1); } } ", "const Foo = class { foo() { alert(1); } } ", ); - - test_script("function foo() { 'use strict';}", "function foo() {}"); } }