if statement wrongly removed when var declaration inside conditional block

[rbellens]

It seems that when you declare and initialize a variable in a code block that executes conditionally, it is assumed that the variable will have this value also outside of the code block. This is wrong when the conditional code block does not get executed, in which case the variable will be undefined. As a result, further optimizations, like removing if statements, produce wrong results.

E.g., the following code produces a wrong result

function x(v) {
  if (v) var w = true;
  if (w) console.log("hello",v);
}

x(false);
x(true);

It produces this:

function x(a){if(a);console.log("hello",a)}x(!1),x(!0);

Which, when executed, outputs:

hello false
hello true

while it should only output the second line.

A related problem is with this code:

function x(v) {
  if (v) {var w = [];for (let i=0;i<1;i++) w.push("hello");}
  if (w) for (let i=0;i<w.length;i++) console.log(w[i],v);
}

x(false);
x(true);

Which produces:

function x(a){if(a){var b=[];for(let a=0;1>a;a++)b.push("hello")}for(let c=0;c<b.length;c++)console.log(b[c],a)}x(!1),x(!0);

When executed this throws an exception Cannot read property 'length' of undefined.

However, unlike the previous example, when disabling the minify-dead-code-elimination plugin, this produces correct code.

The latter example is what happens for example when processing the firebase scripts (see: https://github.com/Polymer/polymer-cli/issues/701)

[vigneshshanmugam]

Looks like a bug. You can workaround by declaring the var manually on top.

function x(v) {
  var w;
  if (v) w = true;
  if (w) console.log("hello",v);
}

This should fix both the issues and produce correct output.

Another option is to disable DCE and simplify in the options, this will result in increased o/p though

{
   deadcode: false,
   simplify: false
}

[j-f1]

@vigneshshanmugam Maybe there should be a transform to pull var declarations to the top of the scope with babel-helper-hoist-variables.

[vigneshshanmugam]

yeah that would be an option, we can fix some of the scoping issues with es2015. Will give it a try.