Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/oxc_minifier/docs/ASSUMPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,14 @@ const v = [];
class A extends v {} // TypeError
```

### No Direct `eval` or `Function` Constructor
### Variables declared in direct `eval` are not referenced outside the eval

Code doesn't dynamically evaluate strings as code. We intend to change this assumption to optional in the future.
Variables declared in direct `eval` are not referenced outside the eval, which is only allowed in non-strict mode.

```javascript
// The minifier assumes this never happens:
eval('var x = 1');
new Function('return x');
console.log(x); // 1
```

### No side effects from accessing to a global variable named `arguments`
Expand Down
49 changes: 25 additions & 24 deletions crates/oxc_minifier/src/peephole/minimize_statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1131,10 +1131,9 @@ impl<'a> PeepholeOptimizations {
ctx: &Ctx<'a, '_>,
non_scoped_literal_only: bool,
) -> bool {
// TODO: we should skip this compression when direct eval exists
// because the code inside eval may reference the variable

if Self::keep_top_level_var_in_script_mode(ctx) {
if Self::keep_top_level_var_in_script_mode(ctx)
|| ctx.current_scope_flags().contains_direct_eval()
{
return false;
}

Expand Down Expand Up @@ -1169,30 +1168,32 @@ impl<'a> PeepholeOptimizations {
declarations: &mut Vec<'a, VariableDeclarator<'a>>,
ctx: &Ctx<'a, '_>,
) -> bool {
// TODO: we should skip this compression when direct eval exists
// because the code inside eval may reference the variable
if Self::keep_top_level_var_in_script_mode(ctx)
|| ctx.current_scope_flags().contains_direct_eval()
|| kind.is_using()
{
return false;
}

let mut changed = false;
if !Self::keep_top_level_var_in_script_mode(ctx) && !kind.is_using() {
let mut i = 1;
while i < declarations.len() {
let (prev_decls, [decl, ..]) = declarations.split_at_mut(i) else { unreachable!() };
let Some(decl_init) = &mut decl.init else {
i += 1;
continue;
};
let old_len = prev_decls.len();
let new_len = Self::substitute_single_use_symbol_in_expression_from_declarators(
decl_init, prev_decls, ctx, false,
);
if old_len != new_len {
changed = true;
let drop_count = old_len - new_len;
declarations.drain(i - drop_count..i);
i -= drop_count;
}
let mut i = 1;
while i < declarations.len() {
let (prev_decls, [decl, ..]) = declarations.split_at_mut(i) else { unreachable!() };
let Some(decl_init) = &mut decl.init else {
i += 1;
continue;
};
let old_len = prev_decls.len();
let new_len = Self::substitute_single_use_symbol_in_expression_from_declarators(
decl_init, prev_decls, ctx, false,
);
if old_len != new_len {
changed = true;
let drop_count = old_len - new_len;
declarations.drain(i - drop_count..i);
i -= drop_count;
}
i += 1;
}
changed
}
Expand Down
7 changes: 6 additions & 1 deletion crates/oxc_minifier/src/peephole/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,11 @@ impl<'a> Normalize {
fn convert_const_to_let(decl: &mut VariableDeclaration<'a>, ctx: &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.scoping().root_scope_id() {
if decl.kind.is_const()
&& ctx.current_scope_id() != ctx.scoping().root_scope_id()
// direct eval may have a assignment inside
&& !ctx.current_scope_flags().contains_direct_eval()
{
let all_declarations_are_only_read =
decl.declarations.iter().flat_map(|d| d.id.get_binding_identifiers()).all(|id| {
ctx.scoping()
Expand Down Expand Up @@ -429,6 +433,7 @@ mod test {
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_same("{ const x = 1; eval('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] }");
Expand Down
14 changes: 12 additions & 2 deletions crates/oxc_minifier/src/peephole/remove_unused_declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ impl<'a> PeepholeOptimizations {
}
let Some(id) = &f.id else { return };
let Some(symbol_id) = id.symbol_id.get() else { return };
if Self::keep_top_level_var_in_script_mode(ctx) {
if Self::keep_top_level_var_in_script_mode(ctx)
|| ctx.current_scope_flags().contains_direct_eval()
{
return;
}
if !ctx.scoping().symbol_is_unused(symbol_id) {
Expand All @@ -69,7 +71,9 @@ impl<'a> PeepholeOptimizations {
}
let Some(id) = &c.id else { return };
let Some(symbol_id) = id.symbol_id.get() else { return };
if Self::keep_top_level_var_in_script_mode(ctx) {
if Self::keep_top_level_var_in_script_mode(ctx)
|| ctx.current_scope_flags().contains_direct_eval()
{
return;
}
if !ctx.scoping().symbol_is_unused(symbol_id) {
Expand Down Expand Up @@ -129,11 +133,17 @@ mod test {
test_options("function foo() {}", "", &options);
test_same_options("function foo() { bar } foo()", &options);
test_same_options("export function foo() {} foo()", &options);
test_same_options("function foo() { bar } eval('foo()')", &options);
}

#[test]
fn remove_unused_class_declaration() {
let options = CompressOptions::smallest();
test_options("class C {}", "", &options);
test_same_options("export class C {}", &options);
test_options("class C {} C", "", &options);
test_same_options("class C {} eval('C')", &options);

// extends
test_options("class C {}", "", &options);
test_options("class C extends Foo {}", "Foo", &options);
Expand Down
5 changes: 4 additions & 1 deletion crates/oxc_minifier/src/peephole/remove_unused_expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,9 @@ impl<'a> PeepholeOptimizations {
else {
return false;
};
if Self::keep_top_level_var_in_script_mode(ctx) {
if Self::keep_top_level_var_in_script_mode(ctx)
|| ctx.current_scope_flags().contains_direct_eval()
{
return false;
}
let reference_id = ident.reference_id();
Expand Down Expand Up @@ -1076,6 +1078,7 @@ mod test {
let options = CompressOptions::smallest();
test_options("var x = 1; x = 2;", "", &options);
test_options("var x = 1; x = foo();", "foo()", &options);
test_same_options("var x = 1; x = 2, eval('x')", &options);
test_same_options("export var foo; foo = 0;", &options);
test_same_options("var x = 1; x = 2, foo(x)", &options);
test_same_options("function foo() { return t = x(); } foo();", &options);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ impl<'a> PeepholeOptimizations {
/// This function uses the private member usage collected during the main traverse
/// to remove unused private fields and methods from the class body.
pub fn remove_unused_private_members(body: &mut ClassBody<'a>, ctx: &mut Ctx<'a, '_>) {
if ctx.current_scope_flags().contains_direct_eval() {
return;
}

let old_len = body.body.len();
body.body.retain(|element| match element {
ClassElement::PropertyDefinition(prop) => {
Expand Down Expand Up @@ -113,6 +117,7 @@ mod test {
"class C { public = 1; #used = 3; method() { return this.public + this.#used; } } new C();",
);
test_same("class C { #unused = foo(); method() { return 1; } } new C();");
test_same("class C { #used = 1; method() { return eval('this.#used'); } } new C();");
}

#[test]
Expand All @@ -129,6 +134,9 @@ mod test {
test_same(
"class C { #helper() { return 1; } method() { return this.#helper(); } } new C();",
);
test_same(
"class C { #helper() { return 1; } method() { return eval('this.#helper()'); } } new C();",
);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ fn test_keep_names(source_text: &str, expected: &str) {
fn test_inline_single_use_variable() {
test_same("function wrapper(arg0, arg1) {using x = foo; return x}");
test_same("async function wrapper(arg0, arg1) { await using x = foo; return x}");
test_same("function wrapper(arg0) { eval('x'); var x = arg0; return x }");

test(
"
Expand Down Expand Up @@ -251,6 +252,8 @@ fn test_inline_past_readonly_variable() {

#[test]
fn test_within_same_variable_declarations() {
test_same("function wrapper() { eval('a'); for (var a = foo, b = a; bar;) return b }");

test_script(
"var a = foo, b = a; for (; bar;) console.log(b)",
"for (var a = foo, b = a; bar;) console.log(b)",
Expand Down
8 changes: 4 additions & 4 deletions tasks/track_memory_allocations/allocs_parser.snap
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
File | File size || Sys allocs | Sys reallocs || Arena allocs | Arena reallocs | Arena bytes
-------------------------------------------------------------------------------------------------------------------------------------------
checker.ts | 2.92 MB || 10157 | 21 || 268665 | 23341
checker.ts | 2.92 MB || 10163 | 21 || 268665 | 23341

cal.com.tsx | 1.06 MB || 2197 | 54 || 138188 | 13712
cal.com.tsx | 1.06 MB || 2205 | 61 || 138188 | 13712

RadixUIAdoptionSection.jsx | 2.52 kB || 1 | 0 || 365 | 66

pdf.mjs | 567.30 kB || 683 | 71 || 90678 | 8148
pdf.mjs | 567.30 kB || 701 | 75 || 90678 | 8148

antd.js | 6.69 MB || 6938 | 236 || 528505 | 55357
antd.js | 6.69 MB || 6995 | 235 || 528505 | 55357

binder.ts | 193.08 kB || 537 | 7 || 16807 | 1475

Loading