Skip to content

fix for issue #56 - #57

Closed
yinwang0 wants to merge 1 commit into
masterfrom
dont-remove-single-statement-blocks
Closed

fix for issue #56#57
yinwang0 wants to merge 1 commit into
masterfrom
dont-remove-single-statement-blocks

Conversation

@yinwang0

Copy link
Copy Markdown

No description provided.

@michaelficarra

Copy link
Copy Markdown
Member

This is far too simplistic of a fix. We should ignore only the potentially problematic locations, and only if a flag is enabled.

@yinwang0

Copy link
Copy Markdown
Author

It is simple because an ideal language won't allow any single-statement blocks without braces. It's just confusing. Removing the braces can't make the code much shorter.

@michaelficarra

Copy link
Copy Markdown
Member

We're not talking about an ideal language here. What is the size difference for AngularJS with/without the braces?

@yinwang0

Copy link
Copy Markdown
Author

Here are some numbers on the current angular.js

total size: 949544
spaceless size: 245547
minified size: 244672
braces count: 10650
single blocks: 2138
space / total: 74.14%
minify-reduction / total: 74.23%
minify-reduction-all / spaceless: 0.73%
minify-reduction-retain-single-blocks / spaceless: 0.36%
braces / total: 1.12%
braces / spacelss: 4.34%
single / total: 0.23%
single / spaceless: 0.87%

Minus the single-statement blocks which already omitted braces, the total saving of removing braces around single statements will be less than 0.23% (counting space), or 0.87% (not counting space). Either way it's negligible.

Plus in ES6 braces matter to the semantics, so it's unsound to remove braces.

I have also noticed that with the spaceless code generated by codegen, the minifier can only reduce its size by 0.73%. If I turn off RemoveSingleStatementBlocks the reduction is 0.36%. So indeed braces are a large portion to the minifier reduction, but the difference doesn't matter much because the reduction is negligible after removing all the spaces.

But any way, I found that I can just skip RemoveSingleStatementBlocks using one of the configurable interfaces, so no need for this pull request.

@michaelficarra
michaelficarra deleted the dont-remove-single-statement-blocks branch April 5, 2016 23:11
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