From a80460c6fe4ead1506f59f03274b5dc993f2efe5 Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Sat, 11 Jan 2025 01:12:09 +0000 Subject: [PATCH] fix(minifier): correctly set `self.changed` when minimizing if stmts (#8420) Before: ``` ==== Input ==== require('./index.js')(function (e, os) { if (e) return console.log(e) return console.log(JSON.stringify(os)) }) ==== First Minification require("./index.js")(function(e, os) { return console.log(e ? e : JSON.stringify(os)); }); ==== Second Minification ==== require("./index.js")(function(e, os) { return console.log(e || JSON.stringify(os)); }); same = false ``` After: ``` ==== Input ==== require('./index.js')(function (e, os) { if (e) return console.log(e) return console.log(JSON.stringify(os)) }) ==== First Minification ==== require("./index.js")(function(e, os) { return console.log(e || JSON.stringify(os)); }); ==== Second Minification ==== require("./index.js")(function(e, os) { return console.log(e || JSON.stringify(os)); }); same = true ``` --- .../peephole_minimize_conditions.rs | 2 ++ crates/oxc_minifier/tests/ast_passes/mod.rs | 20 +++++++++++++++++++ crates/oxc_minifier/tests/mod.rs | 6 +++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs index 93446d745f3a5..32c2fe76afebf 100644 --- a/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs +++ b/crates/oxc_minifier/src/ast_passes/peephole_minimize_conditions.rs @@ -34,6 +34,7 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions { ctx: &mut TraverseCtx<'a>, ) { self.try_replace_if(stmts, ctx); + let changed = self.changed; while self.changed { self.changed = false; self.try_replace_if(stmts, ctx); @@ -41,6 +42,7 @@ impl<'a> Traverse<'a> for PeepholeMinimizeConditions { stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_))); } } + self.changed = self.changed || changed; } fn exit_statement(&mut self, stmt: &mut Statement<'a>, ctx: &mut TraverseCtx<'a>) { diff --git a/crates/oxc_minifier/tests/ast_passes/mod.rs b/crates/oxc_minifier/tests/ast_passes/mod.rs index f0d68f3529e73..7deb1896faa30 100644 --- a/crates/oxc_minifier/tests/ast_passes/mod.rs +++ b/crates/oxc_minifier/tests/ast_passes/mod.rs @@ -1,6 +1,7 @@ mod dead_code_elimination; use oxc_minifier::CompressOptions; +use oxc_span::SourceType; fn test(source_text: &str, expected: &str) { let options = CompressOptions::default(); @@ -11,6 +12,15 @@ fn test_same(source_text: &str) { test(source_text, source_text); } +fn test_idempotent(source: &str, expected: &str) { + let expected = crate::run(expected, SourceType::default(), None); + let first = crate::run(source, SourceType::default(), Some(CompressOptions::default())); + let second = crate::run(&first, SourceType::default(), Some(CompressOptions::default())); + + assert_eq!(second, expected, "\nfor source\n{source}\nexpect\n{expected}\ngot\n{second}"); + assert_eq!(first, second); +} + // Oxc Integration Tests #[test] @@ -30,6 +40,16 @@ fn integration() { } }", ); + + test_idempotent( + "require('./index.js')(function (e, os) { + if (e) return console.log(e) + return console.log(JSON.stringify(os)) + })", + r#"require("./index.js")(function(e, os) { + return console.log(e || JSON.stringify(os)); + });"#, + ); } #[test] // https://github.com/oxc-project/oxc/issues/4341 diff --git a/crates/oxc_minifier/tests/mod.rs b/crates/oxc_minifier/tests/mod.rs index accb6a370f575..09807d00eb8db 100644 --- a/crates/oxc_minifier/tests/mod.rs +++ b/crates/oxc_minifier/tests/mod.rs @@ -15,7 +15,11 @@ pub(crate) fn test(source_text: &str, expected: &str, options: CompressOptions) assert_eq!(result, expected, "\nfor source\n{source_text}\nexpect\n{expected}\ngot\n{result}"); } -fn run(source_text: &str, source_type: SourceType, options: Option) -> String { +pub(crate) fn run( + source_text: &str, + source_type: SourceType, + options: Option, +) -> String { let allocator = Allocator::default(); let ret = Parser::new(&allocator, source_text, source_type).parse(); let mut program = ret.program;