HTML minification and ng-templates #3

Closed
hkdobrev opened this Issue Dec 13, 2012 · 12 comments

3 participants

@hkdobrev

I don't know if HTML minification is or it will ever be in the scope of this project. But there are some issues with HTML minification specific to AngularJS applications.

Currently I am using yeoman for minification of my AngularJS templates and other HTML. I am also embedding my templates in script tags like so:

<script type="text/ng-template">
<p>Some HTML to be minified</p>
...
</script>

Because the contents of a script tag are not minified, I am ending with most of my HTML not minified.

Do you have any plans or suggestions for tackling this problem?

@Munter

I have made sure that AngularJS templates are properly modeled in assetgraph, which means that they will be properly minified when doing a production build as well.
You can try it out using grunt-reduce, a wrapper I wrote to make the build system more compatible with the grunt workflow.

I would argue that ngmin should keep its focus narrow towards rewriting the js to be minifiable by closure compiler, uglify etc. Template minification is a bit far away from that.

@btford
Owner

I agree, though one interesting consideration is renaming directives. I'm not too familiar with assetgraph, but is it within the bounds of its capabilities, @Munter?

Not sure if any of that should be in ngmin or another separate util; I'll have to take a closer look at assetgraph.

@Munter

@btford It might not be exactly within the bounds of assetgraph, since that is a very generalized build system toolkit. But it is a perfect fit for the assetgraph-builder project, a more opinionated implementation including some less general transformations very specificly targeted at building web applications.

We already have some AngularJS support by modelling the templates. And we also already do AST walking to do it:
https://github.com/One-com/assetgraph/blob/master/lib/assets/JavaScript.js#L168-L246

I've already created an issue to try and implement ngmin in assetgraph-builder: assetgraph/assetgraph-builder#38

I would inject a conditional before this line, that runs ngmin if we determine that the graph contains an angular application:
https://github.com/One-com/assetgraph-builder/blob/master/lib/transforms/buildProduction.js#L72

@Munter

Oh, I think I misunderstood directive renaming, now that I read a bit more about it. So the problem with directive renaming is that directives are referenced in the binding attributes in templates?

If so that poses a more difficult problem. Setting up the relations with assetgraph won't be that big of a problem, since the whole point of assetgraph is that every assets is in scope at the same time and you can use all your favorite tools on the raw source. So you could query select any element with a data binding attribute in any html/template file and find the directive names from that. I guess to set up the reference in the graph model, we would then have to walk the JS AST and match directive names. Also doable.

The thing is that currently minification of javascript is handed off to uglify (yui or closure-compiler optionally), which doesn't take any of those relations into account. I'm not sure if we could take the minified code and assume an identical AST and then reverse engineer the renamed directives and reflect them in the templates.

@papandreou do you have any ideas here?

@Munter

I implemented ngmin into an assetgraph transform that prepares the code base for minification:Munter/assetgraph-builder@d6a411f

@btford
Owner

Very nice, @Munter. Thank you so much for your contributions. I really appreciate it. :)

Yeah, that's the issue with renaming directives, and I'm not sure if renaming directives is even worth the trouble at this point.

@Munter

Aren't directive names defined as strings in an angular.directive('name')call?

If so, minification is not an issue at all. Just detect the directive name from any of these structures

<span ng:bind="name">
<span ng_bind="name">
<span ng-bind="name">
<span data-ng-bind="name">
<span x-ng-bind="name">

All of these can be found in html documents using querySelectorAll, like in https://github.com/Munter/assetgraph-builder/blob/ngmin/lib/transforms/angularPreMinification.js#L35-L36

From there it's just walking the AST until finding directive('name') for each of the found directives.

I doubt it will save much though.

@hkdobrev
@Munter

@hkdobrev It's a solved problem. Use grunt-reduce or assetgraph-builder if you want more lowlevel control

@Munter

@btford We ended up reimplementing the ngmin functionality with an uglifyjs AST walker instead of using ngmin. Since we already have the AST of each asset in the graph at the time of preprocessing this ended up being a factor 20 faster. Munter/assetgraph-builder@756c9ce

I'll keep an eye on any changes you make here so we can reflect them in assetgraph-builder asap.

@btford
Owner

@Munter Great! I really appreciate all of the work you've done to make this work. I'll be taking a closer look at asset graph now that I have some free time.

@btford
Owner

Marking as #wontfix for now. It might be possible to come back to this at a later time.

@btford btford closed this Feb 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment