Skip to content
New issue

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

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

Already on GitHub? # to your account

fix builtins plugins from leaking vars #472

Merged
merged 6 commits into from
Mar 22, 2017

Conversation

vigneshshanmugam
Copy link
Member

@vigneshshanmugam vigneshshanmugam commented Mar 9, 2017

fix #461

  • The implementation decides where to insert the created identifier based on the input code.

Even after fixing this, the mangler doesn't rename the identifier properly. Related - #425

const { start: nodeStart } = firstPath.parent;
const { end: nodeEnd } = lastPath.parent;

program.traverse({
Copy link
Member

Choose a reason for hiding this comment

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

the same can be achieved by getFunctionParent recursively till program. program.traverse inside a loop will be costly.

@boopathi boopathi added the Tag: Bug Fix Pull Request fixes a bug label Mar 10, 2017
@vigneshshanmugam
Copy link
Member Author

vigneshshanmugam commented Mar 10, 2017

[Update]

Running mangler on exit fixes #425.

@vigneshshanmugam
Copy link
Member Author

vigneshshanmugam commented Mar 15, 2017

Still leaks when there is no top level IIFE and functions are at same level. Trying out some stuffs.

@codecov
Copy link

codecov bot commented Mar 20, 2017

Codecov Report

Merging #472 into master will increase coverage by 0.11%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master     #472      +/-   ##
==========================================
+ Coverage   82.38%   82.49%   +0.11%     
==========================================
  Files          35       35              
  Lines        2611     2634      +23     
  Branches      923      930       +7     
==========================================
+ Hits         2151     2173      +22     
- Misses        280      281       +1     
  Partials      180      180
Impacted Files Coverage Δ
...ages/babel-plugin-minify-mangle-names/src/index.js 76.39% <100%> (ø) ⬆️
packages/babel-plugin-minify-builtins/src/index.js 92.04% <93.75%> (+1.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2379fc1...f8f5609. Read the comment docs.

@vigneshshanmugam
Copy link
Member Author

vigneshshanmugam commented Mar 20, 2017

Works now for functions at same level.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Tag: Bug Fix Pull Request fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

minify-builtins leaks vars
2 participants