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

Fixes bug that was causing prefix to be added multiple times #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

joaosamouco
Copy link
Contributor

This bug was introduced by our PR #18. It was causing the namePrefix to be added multiple times to renamed variables in some scenarios (ie: IIFE).

Here is an example of what was happening:

(function () {
  var variableA = 1;
  var variableB = 2;
  var variableC = 3;
})();

When we mangled the tree with _ as namePrefix:

(function () {
  var _a = 1;
  var __b = 2;
  var ___c = 3;
})();

@michaelficarra
Copy link
Member

What happens when your prefix is "a"?

@joaosamouco
Copy link
Contributor Author

Just tested it and this is the output of the same example with a as prefix.

(function () {
  var aa = 1;
  var ab = 2;
  var ac = 3;
}());

@@ -79,6 +79,9 @@
prefix = '';
}
do {
if (tip.indexOf(prefix) === 0) {
Copy link
Member

@michaelficarra michaelficarra Apr 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely this could be moved before the loop.

…oked Function Expressions (IIFE) with multiple variable declarations
@joaosamouco
Copy link
Contributor Author

Took your comment in to consideration and moved the condition outside of the loop.

Tell me if you find something else that can be improved.

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

Successfully merging this pull request may close these issues.

2 participants