babel / minify

:scissors: An ES6+ aware minifier based on the Babel toolchain (beta)
https://babeljs.io/repl
MIT License
4.39k stars 225 forks source link

Improve replace + DCE w. surrounding blocks #287

Open kangax opened 7 years ago

kangax commented 7 years ago

When Babili is run with replacements, such as to replace console.log with empty function, but there's surrounding block, that block is never removed:

if (!this.smth) {
  console.log("foo bar");
}
kangax commented 7 years ago

This really comes down to DCE not removing useless (to an extent) statements:

!function(){ smth }()

Since we don't account for getters at the moment (but we could, via option), this should be removed. Once we do that, the original snippet will be completely eliminated too, once it turns from if (!this.smth) { <removed expression> } to !this.smth.

kangax commented 7 years ago

So our DCE looks at expression statement and if it sees identifier, it only removes it if it's a declared one...

!function(x){ x } // <-- removed
!function(){ x } // <-- not removed

Is the motivation here that someone might be accessing an externally declared var or something like that? But then how would that binding have side effects unless it's declared as a getter on global object (Proxies aside)?

Or is the motivation to avoid changing behavior in case of undeclared vars?

!function(){ var x; x }(); /* no error */ --> `function(){ var x; }() /* no error */
!function(){ x } /* possibly error */ --> `!function(){}()` /* no error */

In any case, we should add an option to consider undeclared identifiers effect-less, so that we can minify cases like this.

We should also add an option to consider or disregard getters, so that we can get rid of member expressions as in the original example.

Thoughts?

boopathi commented 7 years ago

I'm sure that we can remove side-effect free expression statements - where Identifier is one of them. And an option to consider if we consider these to cause side-effects -

  1. MemberExpressions - getters and setters - option: pureGetters?
  2. instanceOf
  3. conversion to primitive values (+num,${num}, num + "" etc...)

Point 3 is the tricky one - and I think we should ask the user the disable the entire plugin instead or provide an option ...

kangax commented 7 years ago

Agree about #3.

So let's do this:

boopathi commented 7 years ago

Symbol.hasInstance -

kangax commented 7 years ago

Oh, you mean that we can't remove expression with instanceOf due to side effects? Yeah, this is definitely not a safe transformation (just like string conversion) although probably rarely used one in general code.