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

Constants defined before a while-loop are removed or moved out of scope #801

Closed
rykdesjardins opened this issue Feb 21, 2018 · 2 comments · Fixed by #826
Closed

Constants defined before a while-loop are removed or moved out of scope #801

rykdesjardins opened this issue Feb 21, 2018 · 2 comments · Fixed by #826
Labels
bug Confirmed bug has PR Has an open PR that fixes this issue

Comments

@rykdesjardins
Copy link

rykdesjardins commented Feb 21, 2018

I saw this was fixed and merged in #713, but I can't seem to make it work. Using version 0.3.0 from npm, on Linux.

The smallest code snippet I could come up with is the following.

const array = [];
let loopvar = true;
while (loopvar) { loopvar = false }
array.push(0);

REPL - Try it here

To reproduce, we need a variable defined before a while loop, a while loop, and to refer to the previously defined variable.

Actual Output

// Minified
for(var loopvar=!0;loopvar;)loopvar=!1;array.push(0);

// Manually unminified
for(var loopvar=!0;loopvar;) {
  loopvar=!1;
}
array.push(0);

Expected Output

Something similar to this.

var array = [];
for(var loopvar=!0;loopvar;) {
  loopvar=!1;
}
array.push(0);

Details

For the babelrc, I'm only using two presets.

{
  "presets": ["minify", "es2015"]
}

I believe this is coming from the following sub-package (plugin) : https://github.com/babel/minify/tree/master/packages/babel-plugin-transform-merge-sibling-variables. Might take a look at it and see if I can actually contribute and fix.

The most basic npm script I could come up with is babel dev.js -o build.js. For now, I'm using a simple workaround where I define the array in a higher scope, but it's bad programming. I was thinking of maybe using a reducer instead, but I figured this was worth opening an issue anyways.

For a concrete test, try cloning this repo https://github.com/rykdesjardins/lilium-text, run npm install && npm run build, and open the build/liliumtext.js file. You can find another of a similar issue example if you search for the function called createSelectionContext under LiliumText.

Let me know if you need any additional information!

rykdesjardins added a commit to rykdesjardins/lilium-text that referenced this issue Feb 21, 2018
Will be added back when/if babel/minify#801 is
fixed.
@rykdesjardins rykdesjardins changed the title Constants defined before a while-loop are moved out of scope Constants defined before a while-loop are removed or moved out of scope Feb 28, 2018
@vigneshshanmugam
Copy link
Member

We might need to backport the PR(#826 ) to 0.3.x versions since 0.4.0 uses scoped packages.

@vigneshshanmugam vigneshshanmugam added has PR Has an open PR that fixes this issue bug Confirmed bug labels Apr 30, 2018
@rykdesjardins
Copy link
Author

@vigneshshanmugam Quick comment to let you know I appreciate what you guys are doing. I feel the open source community doesn't thank the brightest minds like you enough for all the innovation.

Sending love from Canada. Thanks for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug has PR Has an open PR that fixes this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants