Skip to content

Commit

Permalink
feat(minifier): replace const with let for non-exported read-only…
Browse files Browse the repository at this point in the history
… variables
  • Loading branch information
sapphi-red committed Jan 26, 2025
1 parent 899d108 commit a40e368
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 15 deletions.
30 changes: 30 additions & 0 deletions crates/oxc_ast/src/ast_impl/js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions crates/oxc_minifier/src/compressor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
11 changes: 7 additions & 4 deletions crates/oxc_minifier/src/peephole/minimize_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }",
Expand Down Expand Up @@ -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; }",
);
}

Expand All @@ -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); }",
);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_minifier/src/peephole/minimize_exit_points.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down
43 changes: 42 additions & 1 deletion crates/oxc_minifier/src/peephole/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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`
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -179,14 +208,26 @@ impl<'a> Normalize {

#[cfg(test)]
mod test {
use crate::tester::test;
use crate::tester::{test, test_same};

#[test]
fn test_while() {
// Verify while loops are converted to FOR loops.
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");
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_minifier/src/peephole/statement_fusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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; } } }",
Expand Down
6 changes: 3 additions & 3 deletions tasks/minsize/minsize.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit a40e368

Please sign in to comment.