Skip to content

Commit e0117db

Browse files
committed
feat(minifier): replace const with let for non-exported read-only variables (#8733)
Replace `const` with `let` when that value does not have any assignments and not exposed.
1 parent dc0b0f2 commit e0117db

File tree

8 files changed

+95
-19
lines changed

8 files changed

+95
-19
lines changed

crates/oxc_ast/src/ast_impl/js.rs

+30
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,11 @@ impl<'a> BindingPattern<'a> {
947947
pub fn get_binding_identifier(&self) -> Option<&BindingIdentifier<'a>> {
948948
self.kind.get_binding_identifier()
949949
}
950+
951+
#[allow(missing_docs)]
952+
pub fn get_binding_identifiers(&self) -> std::vec::Vec<&BindingIdentifier<'a>> {
953+
self.kind.get_binding_identifiers()
954+
}
950955
}
951956

952957
impl<'a> BindingPatternKind<'a> {
@@ -968,6 +973,31 @@ impl<'a> BindingPatternKind<'a> {
968973
}
969974
}
970975

976+
fn append_binding_identifiers<'b>(
977+
&'b self,
978+
idents: &mut std::vec::Vec<&'b BindingIdentifier<'a>>,
979+
) {
980+
match self {
981+
Self::BindingIdentifier(ident) => idents.push(ident),
982+
Self::AssignmentPattern(assign) => assign.left.kind.append_binding_identifiers(idents),
983+
Self::ArrayPattern(pattern) => pattern
984+
.elements
985+
.iter()
986+
.filter_map(|item| item.as_ref())
987+
.for_each(|item| item.kind.append_binding_identifiers(idents)),
988+
Self::ObjectPattern(pattern) => pattern.properties.iter().for_each(|item| {
989+
item.value.kind.append_binding_identifiers(idents);
990+
}),
991+
}
992+
}
993+
994+
#[allow(missing_docs)]
995+
pub fn get_binding_identifiers(&self) -> std::vec::Vec<&BindingIdentifier<'a>> {
996+
let mut idents = vec![];
997+
self.append_binding_identifiers(&mut idents);
998+
idents
999+
}
1000+
9711001
#[allow(missing_docs)]
9721002
pub fn is_destructuring_pattern(&self) -> bool {
9731003
match self {

crates/oxc_minifier/src/compressor.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ impl<'a> Compressor<'a> {
3434
program: &mut Program<'a>,
3535
) {
3636
let mut ctx = ReusableTraverseCtx::new(scopes, symbols, self.allocator);
37-
let normalize_options = NormalizeOptions { convert_while_to_fors: true };
37+
let normalize_options =
38+
NormalizeOptions { convert_while_to_fors: true, convert_const_to_let: true };
3839
Normalize::new(normalize_options, self.options).build(program, &mut ctx);
3940
PeepholeOptimizations::new(self.options.target).run_in_loop(program, &mut ctx);
4041
LatePeepholeOptimizations::new(self.options.target).build(program, &mut ctx);
@@ -53,7 +54,8 @@ impl<'a> Compressor<'a> {
5354
program: &mut Program<'a>,
5455
) {
5556
let mut ctx = ReusableTraverseCtx::new(scopes, symbols, self.allocator);
56-
let normalize_options = NormalizeOptions { convert_while_to_fors: false };
57+
let normalize_options =
58+
NormalizeOptions { convert_while_to_fors: false, convert_const_to_let: false };
5759
Normalize::new(normalize_options, self.options).build(program, &mut ctx);
5860
DeadCodeElimination::new().build(program, &mut ctx);
5961
}

crates/oxc_minifier/src/peephole/minimize_conditions.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -1267,7 +1267,10 @@ mod test {
12671267

12681268
// Dot not fold `let` and `const`.
12691269
// Lexical declaration cannot appear in a single-statement context.
1270-
test_same("if (foo) { const bar = 1 } else { const baz = 1 }");
1270+
test(
1271+
"if (foo) { const bar = 1 } else { const baz = 1 }",
1272+
"if (foo) { let bar = 1 } else { let baz = 1 }",
1273+
);
12711274
test_same("if (foo) { let bar = 1 } else { let baz = 1 }");
12721275
// test(
12731276
// "if (foo) { var bar = 1 } else { var baz = 1 }",
@@ -1401,8 +1404,8 @@ mod test {
14011404
fn test_fold_returns_integration2() {
14021405
// if-then-else duplicate statement removal handles this case:
14031406
test(
1404-
"function test(a) {if (a) {const a = Math.random();if(a) {return a;}} return a; }",
1405-
"function test(a) { if (a) { const a = Math.random(); if (a) return a; } return a; }",
1407+
"function test(a) {if (a) {let a = Math.random();if(a) {return a;}} return a; }",
1408+
"function test(a) { if (a) { let a = Math.random(); if (a) return a; } return a; }",
14061409
);
14071410
}
14081411

@@ -1412,7 +1415,7 @@ mod test {
14121415
// refers to a different variable.
14131416
// We only try removing duplicate statements if the AST is normalized and names are unique.
14141417
test_same(
1415-
"if (Math.random() < 0.5) { const x = 3; alert(x); } else { const x = 5; alert(x); }",
1418+
"if (Math.random() < 0.5) { let x = 3; alert(x); } else { let x = 5; alert(x); }",
14161419
);
14171420
}
14181421

crates/oxc_minifier/src/peephole/minimize_exit_points.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ mod test {
130130
test(
131131
"function f(a) {
132132
if (a) {
133-
const a = Math.random();
133+
let a = Math.random();
134134
if (a < 0.5) {
135135
return a;
136136
}
@@ -139,7 +139,7 @@ mod test {
139139
}",
140140
"function f(a) {
141141
if (a) {
142-
const a = Math.random();
142+
let a = Math.random();
143143
if (a < 0.5) return a;
144144
}
145145
return a;

crates/oxc_minifier/src/peephole/normalize.rs

+42-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use crate::{ctx::Ctx, CompressOptions};
1010
#[derive(Default)]
1111
pub struct NormalizeOptions {
1212
pub convert_while_to_fors: bool,
13+
pub convert_const_to_let: bool,
1314
}
1415

1516
/// Normalize AST
@@ -19,6 +20,7 @@ pub struct NormalizeOptions {
1920
/// * remove `Statement::EmptyStatement`
2021
/// * remove `ParenthesizedExpression`
2122
/// * convert whiles to fors
23+
/// * convert `const` to `let` for non-exported variables
2224
/// * convert `Infinity` to `f64::INFINITY`
2325
/// * convert `NaN` to `f64::NaN`
2426
/// * convert `var x; void x` to `void 0`
@@ -49,6 +51,16 @@ impl<'a> Traverse<'a> for Normalize {
4951
});
5052
}
5153

54+
fn exit_variable_declaration(
55+
&mut self,
56+
decl: &mut VariableDeclaration<'a>,
57+
ctx: &mut TraverseCtx<'a>,
58+
) {
59+
if self.options.convert_const_to_let {
60+
Self::convert_const_to_let(decl, ctx);
61+
}
62+
}
63+
5264
fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) {
5365
match stmt {
5466
Statement::WhileStatement(_) if self.options.convert_while_to_fors => {
@@ -136,6 +148,23 @@ impl<'a> Normalize {
136148
*stmt = Statement::ForStatement(for_stmt);
137149
}
138150

151+
fn convert_const_to_let(decl: &mut VariableDeclaration<'a>, ctx: &mut TraverseCtx<'a>) {
152+
// checking whether the current scope is the root scope instead of
153+
// checking whether any variables are exposed to outside (e.g. `export` in ESM)
154+
if decl.kind.is_const() && ctx.current_scope_id() != ctx.scopes().root_scope_id() {
155+
let all_declarations_are_only_read = decl.declarations.iter().all(|decl| {
156+
decl.id.get_binding_identifiers().iter().all(|id| {
157+
ctx.symbols()
158+
.get_resolved_references(id.symbol_id())
159+
.all(|reference| reference.flags().is_read_only())
160+
})
161+
});
162+
if all_declarations_are_only_read {
163+
decl.kind = VariableDeclarationKind::Let;
164+
}
165+
}
166+
}
167+
139168
/// Transforms `undefined` => `void 0`, `Infinity` => `f64::Infinity`, `NaN` -> `f64::NaN`.
140169
/// So subsequent passes don't need to look up whether these variables are shadowed or not.
141170
fn try_compress_identifier(
@@ -179,14 +208,26 @@ impl<'a> Normalize {
179208

180209
#[cfg(test)]
181210
mod test {
182-
use crate::tester::test;
211+
use crate::tester::{test, test_same};
183212

184213
#[test]
185214
fn test_while() {
186215
// Verify while loops are converted to FOR loops.
187216
test("while(c < b) foo()", "for(; c < b;) foo()");
188217
}
189218

219+
#[test]
220+
fn test_const_to_let() {
221+
test_same("const x = 1"); // keep top-level (can be replaced with "let" if it's ESM and not exported)
222+
test("{ const x = 1 }", "{ let x = 1 }");
223+
test_same("{ const x = 1; x = 2 }"); // keep assign error
224+
test("{ const x = 1, y = 2 }", "{ let x = 1, y = 2 }");
225+
test("{ const { x } = { x: 1 } }", "{ let { x } = { x: 1 } }");
226+
test("{ const [x] = [1] }", "{ let [x] = [1] }");
227+
test("{ const [x = 1] = [] }", "{ let [x = 1] = [] }");
228+
test("for (const x in y);", "for (let x in y);");
229+
}
230+
190231
#[test]
191232
fn test_void_ident() {
192233
test("var x; void x", "var x");

crates/oxc_minifier/src/peephole/statement_fusion.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ mod test {
248248
fn fuse_into_vanilla_for2() {
249249
test("a;b;c;for(var d;g;){}", "a,b,c;for(var d;g;);");
250250
test("a;b;c;for(let d;g;){}", "a,b,c;for(let d;g;);");
251-
test("a;b;c;for(const d = 5;g;){}", "a,b,c;for(const d = 5;g;);");
251+
test("a;b;c;for(const d = 5;g;){}", "a,b,c;for(let d = 5;g;);");
252252
}
253253

254254
#[test]
@@ -292,10 +292,10 @@ mod test {
292292
test("a; {b;}", "a,b");
293293
test("a; {b; var a = 1;}", "{a, b; var a = 1;}");
294294
test_same("a; { b; let a = 1; }");
295-
test_same("a; { b; const a = 1; }");
295+
test("a; { b; const a = 1; }", "a; { b; let a = 1; }");
296296
test_same("a; { b; class a {} }");
297297
test_same("a; { b; function a() {} }");
298-
test_same("a; { b; const otherVariable = 1; }");
298+
test("a; { b; const otherVariable = 1; }", "a; { b; let otherVariable = 1; }");
299299

300300
// test(
301301
// "function f(a) { if (COND) { a; { b; let a = 1; } } }",

crates/oxc_minifier/tests/peephole/esbuild.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,15 @@ fn js_parser_test() {
155155
test("const a=0; while (1) ;", "const a = 0;for (;;) ;");
156156
test("var a; for (var b;;) ;", "for (var a, b;;) ;");
157157
test("let a; for (let b;;) ;", "let a;for (let b;;) ;");
158-
test("const a=0; for (const b = 1;;) ;", "const a = 0;for (const b = 1;;) ;");
158+
test("const a=0; for (const b = 1;;) ;", "const a = 0;for (let b = 1;;) ;");
159159
test("export var a; while (1) ;", "export var a;for (;;) ;");
160160
test("export let a; while (1) ;", "export let a;for (;;) ;");
161161
test("export const a=0; while (1) ;", "export const a = 0;for (;;) ;");
162162
test("export var a; for (var b;;) ;", "export var a;for (var b;;) ;");
163163
test("export let a; for (let b;;) ;", "export let a;for (let b;;) ;");
164-
test("export const a=0; for (const b = 1;;) ;", "export const a = 0;for (const b = 1;;) ;");
164+
test("export const a=0; for (const b = 1;;) ;", "export const a = 0;for (let b = 1;;) ;");
165165
test("var a; for (let b;;) ;", "var a;for (let b;;) ;");
166-
test("let a; for (const b=0;;) ;", "let a;for (const b = 0;;) ;");
166+
test("let a; for (const b=0;;) ;", "let a;for (let b = 0;;) ;");
167167
test("const a=0; for (var b;;) ;", "const a = 0;for (var b;;) ;");
168168
test("a(); while (1) ;", "for (a();;) ;");
169169
test("a(); for (b();;) ;", "for (a(), b();;) ;");
@@ -272,7 +272,7 @@ fn js_parser_test() {
272272
// test("x['-2147483648']", "x[-2147483648];");
273273
test("x['-2147483649']", "x['-2147483649'];");
274274
test("while(1) { while (1) {} }", "for (;;) for (;;) ;");
275-
test("while(1) { const x = y; }", "for (;;) { const x = y;}");
275+
test("while(1) { const x = y; }", "for (;;) { let x = y;}");
276276
test("while(1) { let x; }", "for (;;) { let x;}");
277277
// test("while(1) { var x; }", "for (;;) var x;");
278278
test("while(1) { class X {} }", "for (;;) { class X { }}");

tasks/minsize/minsize.snap

+3-3
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ Original | minified | minified | gzip | gzip | Fixture
1111

1212
544.10 kB | 71.50 kB | 72.48 kB | 25.92 kB | 26.20 kB | lodash.js
1313

14-
555.77 kB | 272.12 kB | 270.13 kB | 88.59 kB | 90.80 kB | d3.js
14+
555.77 kB | 271.68 kB | 270.13 kB | 88.48 kB | 90.80 kB | d3.js
1515

16-
1.01 MB | 458.28 kB | 458.89 kB | 123.94 kB | 126.71 kB | bundle.min.js
16+
1.01 MB | 457.66 kB | 458.89 kB | 123.79 kB | 126.71 kB | bundle.min.js
1717

1818
1.25 MB | 650.70 kB | 646.76 kB | 161.49 kB | 163.73 kB | three.js
1919

20-
2.14 MB | 719.06 kB | 724.14 kB | 162.43 kB | 181.07 kB | victory.js
20+
2.14 MB | 719.00 kB | 724.14 kB | 162.41 kB | 181.07 kB | victory.js
2121

2222
3.20 MB | 1.01 MB | 1.01 MB | 325.38 kB | 331.56 kB | echarts.js
2323

0 commit comments

Comments
 (0)