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

JS Mangle private properties => reduce minified JS by 11.9% (4.8% gzipped) #21715

Closed
wants to merge 1 commit into from

Conversation

pvdlg
Copy link
Contributor

@pvdlg pvdlg commented Jan 13, 2017

All the private attribute (variables or function) in Bootstrap JS modules starts with _ so we can easily configure Uglify to mandle them and produce a small minified JS file.

Minified size Minified gain Minified + zip size Minified + zip gain
Before 46,653b - 12,648b -
After 41,059b 11.9% 12,063b 4.8%

In addition, this size reductions cumulate with #21711

@mdo
Copy link
Member

mdo commented Aug 22, 2017

This needs review and conflicts resolved before we could merge it now. @Johann-S @XhmikosR can you take a look and let me know if we should proceed with this PR or something else?

@XhmikosR
Copy link
Member

@vanduynslagerp: how safe is that?

@XhmikosR
Copy link
Member

BTW @vanduynslagerp you would need to move the change in build/uglifyjs.config.json

@pvdlg
Copy link
Contributor Author

pvdlg commented Aug 22, 2017

@vanduynslagerp: how safe is that?

It's safe as long as you don't prefix your public class methods and properties with _.
And it provides a benefit as long as you do prefix your private class methods and properties with _.

It was the case when I opened this PR.
I didn't contribute to Bootstrap for a while and I didn't follow the recent updates, so I can't tell about the current safety/efficiency of this modification with the current code base.

Feel free to re-use or adapt the modification or close the PR if the code has changed and doesn't follow the convention of prefixing private methods and properties with _.

@XhmikosR
Copy link
Member

@Johann-S: what do you think about this? If you agree we should make a new branch and test this ourselves.

@XhmikosR XhmikosR closed this Aug 22, 2017
@Johann-S
Copy link
Member

Yes sure, our minified file size will grow with the removal off jQuery from our dependencies so optimizations are welcome here 👍

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

Successfully merging this pull request may close these issues.

5 participants