Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected behavior in constant-folding-plugin when using dev=false #208

Open
chrisradek opened this issue Jul 30, 2018 · 3 comments
Open

Comments

@chrisradek
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?

When I declare (using var) a variable inside of a block, then check its 'truthy'-ness in a conditional outside that block, some code paths are removed.

Simplified example:

function foo(Bar) {
  if (typeof Bar != 'undefined') {
    var doesExist = true;
  }

  if (doesExist) {
    return Bar;
  } else {
    console.log('Bar does not exist');
  }
}

The constant-folding-plugin transforms this into:

function foo(Bar) {
  if (typeof Bar != 'undefined') {
    var doesExist = true;
  }

  {
    return Bar;
  }
}

If I move the doesExist declaration to be outside of the if statement, then the plugin outputs what I'd expect.

Updated input:

function foo(Bar) {
  var doesExist;
  if (typeof Bar != 'undefined') {
     doesExist = true;
  }

  if (doesExist) {
    return Bar;
  } else {
    console.log('Bar does not exist');
  }
}

Transformed output:

function foo(Bar) {
  var doesExist;

  if (typeof Bar != 'undefined') {
    doesExist = true;
  }

  if (doesExist) {
    return Bar;
  } else {
    console.log('Bar does not exist');
  }
}

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

What is the expected behavior?
Variables declared with var should be treated initially as undefined within a function scope, even when initialization occurs on the same line. Maybe this would only have to be true when a variable is declared and initialized within a function's inner-block. Given the above input, I would expect the output to look something like:

function foo(Bar) {
  if (typeof Bar != 'undefined') {
    var doesExist = true;
  }

  if (doesExist) {
    return Bar;
  } else {
    console.log('Bar does not exist');
  }
}

Please provide your exact Metro configuration and mention your Metro, node, yarn/npm version and operating system.

Using metro 0.38.3 with react-native 0.56.0. yarn version 1.7.0, npm version 6.2.0, node version 8.11.2.

@rafeca
Copy link
Contributor

rafeca commented Jul 30, 2018

cc/ @mjesun I think you fixed this one, but the fix may not be available on metro 0.38.x

If that's the case, can you check if it's easy to cherry pick?

@mjesun
Copy link
Contributor

mjesun commented Aug 1, 2018

That's a tricky one, and you're right, the removal of the if block in this case is not good. That's a bug in the path.evaluate function used by the constant folding plugin.

I've created an issue in Babel (babel/babel#8411), although it's not the first one I've already opened in regards to wrong evaluations (see babel/babel#8017).

@chrisradek
Copy link
Author

Thanks for the update @mjesun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants