oxc-project / oxc

⚓ A collection of JavaScript tools written in Rust.
https://oxc.rs
MIT License
12.57k stars 460 forks source link

minifier: dce + var hosting edge cases #7209

Open underfin opened 3 weeks ago

underfin commented 3 weeks ago

Input


if (false) {
    for (var a; unknownGlobal && true; unknownGlobal && true) var b;
} else {
    console.log(a, b);
}

if (((() => console.log('effect'))(), true)) {
} else {
    var c = 1;
    for (var c; unknownGlobal && true; unknownGlobal && true) var d;
}
console.log(c, d);

Output

console.log(a, b);
console.log(c, d);

Expected

var b, a; {
    console.log(a, b);
}

var c, d; if (((() => console.log('effect'))(), true)) ;
console.log(c, d);

The a,b,c,d need to hosted, the ((() => console.log('effect'))() need to preserve.

Found the case at https://github.com/rollup/rollup/blob/master/test/form/samples/hoisted-vars-in-dead-branches/main.js

7086cmd commented 2 weeks ago

Is the swc's result better than the expected?

var d, c;
console.log(void 0, void 0), console.log('effect'), console.log(c, d);
Closure Compiler's ```js var d,c,b,a;console.log(a,b);(()=>console.log("effect"))();console.log(c,d); ```
Boshen commented 2 weeks ago

I need to spend some time rewriting this part of the code, it's a bit more involved.

underfin commented 2 days ago

Here find a related interesting test at https://github.com/rollup/rollup/blob/master/test/form/samples/pure-comment-scenarios-complex/main.js#L39-L42. cc @IWANABETHATGUY

Input

/* @__PURE__ */(function(){x})() || true ? foo() : bar();
true || /* @__PURE__ */(function(){y})() ? foo() : bar();
/* @__PURE__ */(function(){x})() && false ? foo() : bar();
false && /* @__PURE__ */(function(){y})() ? foo() : bar();

Output

/* @__PURE__ */(function(){})() || true ? foo() : bar();
foo() ;
/* @__PURE__ */(function(){})() && false ? foo() : bar();
bar();
underfin commented 2 days ago

Add more incomprehensible case at https://github.com/rollup/rollup/blob/master/test/form/samples/nested-pure-comments/main.js#L9. The /*@__PURE__*/remove() ? /*@__PURE__*/remove() : /*@__PURE__*/remove(); is removed at https://github.com/rollup/rollup/blob/master/test/form/samples/nested-pure-comments/_expected.js#L6-L8

IWANABETHATGUY commented 2 days ago

It seems not related to var hoisting,

/* @__PURE__ */(function(){x})() || true ? foo() : bar();
true || /* @__PURE__ */(function(){y})() ? foo() : bar();
/* @__PURE__ */(function(){x})() && false ? foo() : bar();
false && /* @__PURE__ */(function(){y})() ? foo() : bar();

For this case, we have same output as esbuild, so this is expected for now. https://hyrious.me/esbuild-repl/?version=0.23.0&b=e%00entry.js%00%2F*+%40__PURE__+*%2F%28function%28%29%7Bx%7D%29%28%29+%7C%7C+true+%3F+foo%28%29+%3A+bar%28%29%3B%0Atrue+%7C%7C+%2F*+%40__PURE__+*%2F%28function%28%29%7By%7D%29%28%29+%3F+foo%28%29+%3A+bar%28%29%3B%0A%2F*+%40__PURE__+*%2F%28function%28%29%7Bx%7D%29%28%29+%26%26+false+%3F+foo%28%29+%3A+bar%28%29%3B%0Afalse+%26%26+%2F*+%40__PURE__+*%2F%28function%28%29%7By%7D%29%28%29+%3F+foo%28%29+%3A+bar%28%29%3B&b=%00file.js%00&o=%7B%0A++treeShaking%3A+true%2C%0A++external%3A+%5B%22c%22%2C+%22a%22%2C+%22b%22%5D%2C%0A%22bundle%22%3A+true%2C%0Aformat%3A+%22esm%22%2C%0Aminify%3A+true%0A%7D

IWANABETHATGUY commented 2 days ago

Also, this seems not related to this issue, let's discuss it in rolldown issues or dc