From a40e3680474cc76690eded1fe4c561e4f18cc75e Mon Sep 17 00:00:00 2001 From: sapphi-red <49056869+sapphi-red@users.noreply.github.com> Date: Sun, 26 Jan 2025 19:16:04 +0900 Subject: [PATCH] feat(minifier): replace `const` with `let` for non-exported read-only variables --- crates/oxc_ast/src/ast_impl/js.rs | 30 +++++++++++++ crates/oxc_minifier/src/compressor.rs | 6 ++- .../src/peephole/minimize_conditions.rs | 11 +++-- .../src/peephole/minimize_exit_points.rs | 4 +- crates/oxc_minifier/src/peephole/normalize.rs | 43 ++++++++++++++++++- .../src/peephole/statement_fusion.rs | 6 +-- tasks/minsize/minsize.snap | 6 +-- 7 files changed, 91 insertions(+), 15 deletions(-) diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index 789df54419fe8d..63e20fe7dad74d 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -947,6 +947,11 @@ impl<'a> BindingPattern<'a> { pub fn get_binding_identifier(&self) -> Option<&BindingIdentifier<'a>> { self.kind.get_binding_identifier() } + + #[allow(missing_docs)] + pub fn get_binding_identifiers(&self) -> std::vec::Vec<&BindingIdentifier<'a>> { + self.kind.get_binding_identifiers() + } } impl<'a> BindingPatternKind<'a> { @@ -968,6 +973,31 @@ impl<'a> BindingPatternKind<'a> { } } + fn append_binding_identifiers<'b>( + &'b self, + idents: &mut std::vec::Vec<&'b BindingIdentifier<'a>>, + ) { + match self { + Self::BindingIdentifier(ident) => idents.push(ident), + Self::AssignmentPattern(assign) => assign.left.kind.append_binding_identifiers(idents), + Self::ArrayPattern(pattern) => pattern + .elements + .iter() + .filter_map(|item| item.as_ref()) + .for_each(|item| item.kind.append_binding_identifiers(idents)), + Self::ObjectPattern(pattern) => pattern.properties.iter().for_each(|item| { + item.value.kind.append_binding_identifiers(idents); + }), + } + } + + #[allow(missing_docs)] + pub fn get_binding_identifiers(&self) -> std::vec::Vec<&BindingIdentifier<'a>> { + let mut idents = vec![]; + self.append_binding_identifiers(&mut idents); + idents + } + #[allow(missing_docs)] pub fn is_destructuring_pattern(&self) -> bool { match self { diff --git a/crates/oxc_minifier/src/compressor.rs b/crates/oxc_minifier/src/compressor.rs index c9514c26cb5f1a..1537007f02638f 100644 --- a/crates/oxc_minifier/src/compressor.rs +++ b/crates/oxc_minifier/src/compressor.rs @@ -34,7 +34,8 @@ impl<'a> Compressor<'a> { program: &mut Program<'a>, ) { let mut ctx = ReusableTraverseCtx::new(scopes, symbols, self.allocator); - let normalize_options = NormalizeOptions { convert_while_to_fors: true }; + let normalize_options = + NormalizeOptions { convert_while_to_fors: true, convert_const_to_let: true }; Normalize::new(normalize_options, self.options).build(program, &mut ctx); PeepholeOptimizations::new(self.options.target).run_in_loop(program, &mut ctx); LatePeepholeOptimizations::new(self.options.target).build(program, &mut ctx); @@ -53,7 +54,8 @@ impl<'a> Compressor<'a> { program: &mut Program<'a>, ) { let mut ctx = ReusableTraverseCtx::new(scopes, symbols, self.allocator); - let normalize_options = NormalizeOptions { convert_while_to_fors: false }; + let normalize_options = + NormalizeOptions { convert_while_to_fors: false, convert_const_to_let: false }; Normalize::new(normalize_options, self.options).build(program, &mut ctx); DeadCodeElimination::new().build(program, &mut ctx); } diff --git a/crates/oxc_minifier/src/peephole/minimize_conditions.rs b/crates/oxc_minifier/src/peephole/minimize_conditions.rs index f4f18999237222..8fd2bf3a68f3ea 100644 --- a/crates/oxc_minifier/src/peephole/minimize_conditions.rs +++ b/crates/oxc_minifier/src/peephole/minimize_conditions.rs @@ -1267,7 +1267,10 @@ mod test { // Dot not fold `let` and `const`. // Lexical declaration cannot appear in a single-statement context. - test_same("if (foo) { const bar = 1 } else { const baz = 1 }"); + test( + "if (foo) { const bar = 1 } else { const baz = 1 }", + "if (foo) { let bar = 1 } else { let baz = 1 }", + ); test_same("if (foo) { let bar = 1 } else { let baz = 1 }"); // test( // "if (foo) { var bar = 1 } else { var baz = 1 }", @@ -1401,8 +1404,8 @@ mod test { fn test_fold_returns_integration2() { // if-then-else duplicate statement removal handles this case: test( - "function test(a) {if (a) {const a = Math.random();if(a) {return a;}} return a; }", - "function test(a) { if (a) { const a = Math.random(); if (a) return a; } return a; }", + "function test(a) {if (a) {let a = Math.random();if(a) {return a;}} return a; }", + "function test(a) { if (a) { let a = Math.random(); if (a) return a; } return a; }", ); } @@ -1412,7 +1415,7 @@ mod test { // refers to a different variable. // We only try removing duplicate statements if the AST is normalized and names are unique. test_same( - "if (Math.random() < 0.5) { const x = 3; alert(x); } else { const x = 5; alert(x); }", + "if (Math.random() < 0.5) { let x = 3; alert(x); } else { let x = 5; alert(x); }", ); } diff --git a/crates/oxc_minifier/src/peephole/minimize_exit_points.rs b/crates/oxc_minifier/src/peephole/minimize_exit_points.rs index 8d324d9542ea15..04d531ee3b363e 100644 --- a/crates/oxc_minifier/src/peephole/minimize_exit_points.rs +++ b/crates/oxc_minifier/src/peephole/minimize_exit_points.rs @@ -130,7 +130,7 @@ mod test { test( "function f(a) { if (a) { - const a = Math.random(); + let a = Math.random(); if (a < 0.5) { return a; } @@ -139,7 +139,7 @@ mod test { }", "function f(a) { if (a) { - const a = Math.random(); + let a = Math.random(); if (a < 0.5) return a; } return a; diff --git a/crates/oxc_minifier/src/peephole/normalize.rs b/crates/oxc_minifier/src/peephole/normalize.rs index ed887c9d20a727..5a4274c76512ee 100644 --- a/crates/oxc_minifier/src/peephole/normalize.rs +++ b/crates/oxc_minifier/src/peephole/normalize.rs @@ -10,6 +10,7 @@ use crate::{ctx::Ctx, CompressOptions}; #[derive(Default)] pub struct NormalizeOptions { pub convert_while_to_fors: bool, + pub convert_const_to_let: bool, } /// Normalize AST @@ -19,6 +20,7 @@ pub struct NormalizeOptions { /// * remove `Statement::EmptyStatement` /// * remove `ParenthesizedExpression` /// * convert whiles to fors +/// * convert `const` to `let` for non-exported variables /// * convert `Infinity` to `f64::INFINITY` /// * convert `NaN` to `f64::NaN` /// * convert `var x; void x` to `void 0` @@ -49,6 +51,16 @@ impl<'a> Traverse<'a> for Normalize { }); } + fn exit_variable_declaration( + &mut self, + decl: &mut VariableDeclaration<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + if self.options.convert_const_to_let { + Self::convert_const_to_let(decl, ctx); + } + } + fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { match stmt { Statement::WhileStatement(_) if self.options.convert_while_to_fors => { @@ -136,6 +148,23 @@ impl<'a> Normalize { *stmt = Statement::ForStatement(for_stmt); } + fn convert_const_to_let(decl: &mut VariableDeclaration<'a>, ctx: &mut TraverseCtx<'a>) { + // checking whether the current scope is the root scope instead of + // checking whether any variables are exposed to outside (e.g. `export` in ESM) + if decl.kind.is_const() && ctx.current_scope_id() != ctx.scopes().root_scope_id() { + let all_declarations_are_only_read = decl.declarations.iter().all(|decl| { + decl.id.get_binding_identifiers().iter().all(|id| { + ctx.symbols() + .get_resolved_references(id.symbol_id()) + .all(|reference| reference.flags().is_read_only()) + }) + }); + if all_declarations_are_only_read { + decl.kind = VariableDeclarationKind::Let; + } + } + } + /// Transforms `undefined` => `void 0`, `Infinity` => `f64::Infinity`, `NaN` -> `f64::NaN`. /// So subsequent passes don't need to look up whether these variables are shadowed or not. fn try_compress_identifier( @@ -179,7 +208,7 @@ impl<'a> Normalize { #[cfg(test)] mod test { - use crate::tester::test; + use crate::tester::{test, test_same}; #[test] fn test_while() { @@ -187,6 +216,18 @@ mod test { test("while(c < b) foo()", "for(; c < b;) foo()"); } + #[test] + fn test_const_to_let() { + test_same("const x = 1"); // keep top-level (can be replaced with "let" if it's ESM and not exported) + test("{ const x = 1 }", "{ let x = 1 }"); + test_same("{ const x = 1; x = 2 }"); // keep assign error + test("{ const x = 1, y = 2 }", "{ let x = 1, y = 2 }"); + test("{ const { x } = { x: 1 } }", "{ let { x } = { x: 1 } }"); + test("{ const [x] = [1] }", "{ let [x] = [1] }"); + test("{ const [x = 1] = [] }", "{ let [x = 1] = [] }"); + test("for (const x in y);", "for (let x in y);"); + } + #[test] fn test_void_ident() { test("var x; void x", "var x"); diff --git a/crates/oxc_minifier/src/peephole/statement_fusion.rs b/crates/oxc_minifier/src/peephole/statement_fusion.rs index 122a695a5834b5..6601f1bbaa79fb 100644 --- a/crates/oxc_minifier/src/peephole/statement_fusion.rs +++ b/crates/oxc_minifier/src/peephole/statement_fusion.rs @@ -248,7 +248,7 @@ mod test { fn fuse_into_vanilla_for2() { test("a;b;c;for(var d;g;){}", "a,b,c;for(var d;g;);"); test("a;b;c;for(let d;g;){}", "a,b,c;for(let d;g;);"); - test("a;b;c;for(const d = 5;g;){}", "a,b,c;for(const d = 5;g;);"); + test("a;b;c;for(const d = 5;g;){}", "a,b,c;for(let d = 5;g;);"); } #[test] @@ -292,10 +292,10 @@ mod test { test("a; {b;}", "a,b"); test("a; {b; var a = 1;}", "{a, b; var a = 1;}"); test_same("a; { b; let a = 1; }"); - test_same("a; { b; const a = 1; }"); + test("a; { b; const a = 1; }", "a; { b; let a = 1; }"); test_same("a; { b; class a {} }"); test_same("a; { b; function a() {} }"); - test_same("a; { b; const otherVariable = 1; }"); + test("a; { b; const otherVariable = 1; }", "a; { b; let otherVariable = 1; }"); // test( // "function f(a) { if (COND) { a; { b; let a = 1; } } }", diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index ac6019ca73df8e..cf2a03ff0a96d7 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -11,13 +11,13 @@ Original | minified | minified | gzip | gzip | Fixture 544.10 kB | 71.50 kB | 72.48 kB | 25.92 kB | 26.20 kB | lodash.js -555.77 kB | 272.12 kB | 270.13 kB | 88.59 kB | 90.80 kB | d3.js +555.77 kB | 271.68 kB | 270.13 kB | 88.48 kB | 90.80 kB | d3.js -1.01 MB | 458.28 kB | 458.89 kB | 123.94 kB | 126.71 kB | bundle.min.js +1.01 MB | 457.66 kB | 458.89 kB | 123.79 kB | 126.71 kB | bundle.min.js 1.25 MB | 650.70 kB | 646.76 kB | 161.49 kB | 163.73 kB | three.js -2.14 MB | 719.06 kB | 724.14 kB | 162.43 kB | 181.07 kB | victory.js +2.14 MB | 719.00 kB | 724.14 kB | 162.41 kB | 181.07 kB | victory.js 3.20 MB | 1.01 MB | 1.01 MB | 325.38 kB | 331.56 kB | echarts.js