Skip to content

Commit

Permalink
fix(minifier): correctly set self.changed when minimizing if stmts (#…
Browse files Browse the repository at this point in the history
…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
```
  • Loading branch information
camc314 committed Jan 11, 2025
1 parent b29655f commit a80460c
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ 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);
if stmts.iter().any(|stmt| matches!(stmt, Statement::EmptyStatement(_))) {
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>) {
Expand Down
20 changes: 20 additions & 0 deletions crates/oxc_minifier/tests/ast_passes/mod.rs
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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]
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion crates/oxc_minifier/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CompressOptions>) -> String {
pub(crate) fn run(
source_text: &str,
source_type: SourceType,
options: Option<CompressOptions>,
) -> String {
let allocator = Allocator::default();
let ret = Parser::new(&allocator, source_text, source_type).parse();
let mut program = ret.program;
Expand Down

0 comments on commit a80460c

Please sign in to comment.