Minify: Incorrect transformation on React project with both simplify and dead code elimination enabled

[Gustorn]

I couldn't make a minimal repro, but compiling my project with these two plugins enabled leads to the following error message (it's worth mentioning that babili is run on single fairly large file created by Webpack):

Uncaught TypeError: inst.render is not a function        ReactCompositeComponent.js:811

The issue doesn't manifest with either of the plugins disabled. I narrowed the bug to createPrevExpressionEater("if") in simplify whose result is removed by the if (evaluateTest === false) block in the IfStatement part of dead-code-elimination.

If there's any diagnostic I can run to be more helpful I'd be happy to do so.

[jakepusateri]

I'm also seeing something along these lines as well, while trying to minify https://github.com/facebook/react/blob/master/src/isomorphic/modern/class/ReactComponent.js in the context of a webpack module. It ends up causing the error:

Uncaught TypeError: Cannot call a class as a function

for me. I was able to get a more minimal repro of:

Input

function ReactComponent() {
}

ReactComponent.prototype.isReactComponent = {};

if (false) {
  const anyVar = 1;
}

Expected

function ReactComponent(){}ReactComponent.prototype.isReactComponent={};

Actual

function ReactComponent(){};

It appears to work correctly in the REPL at the moment, but not on [email protected] and a babel version of 6.22.2 (babel-core 6.22.1)

[boopathi]

Strange that if (0) { const foo = bar; } is causing the previous statement to be removed.

[Gustorn]

The problem seems to be that dead code elimination ignores the side effects of sequence expressions.
simplify transforms the code provided by @jakepusateri to this:

function ReactComponent() {
}

if (ReactComponent.prototype.isReactComponent = {}, false) {
  const anyVar = 1;
}

which dead code elimination undestands as:

if (false) {
  const anyVar = 1;
}

I'm not terribly familiar with the Babel AST but in light of dead code elimination this transformation only seems safe to me when the node being moved into the if is pure. In fact, replacing this block in createPrevExpressionEater in simplify:

var prev = path.getSibling(path.key - 1);
if (!prev.isExpressionStatement()) {
  return;
}

with

var prev = path.getSibling(path.key - 1);
if (!prev.isExpressionStatement() || !prev.isPure()) {
  return;
}

solves the minimal repro case for me (couldn't test it on my project yet).

[boopathi]

Yes! Exactly.

The order of the transformations again. 0.0.9 didn't have DCE in exit. Now it's in Program.exit. So multipleSequenceExpressions takes place first and then DCE.

isPure check (which is necessary) should fix the bug. But it will result in ...; if (sideEffects, !1) {} ... code, instead of simply - ...; sideEffects; .... I'm trying out ways to eliminate this.