Several performance improvements (ng-if vs. ng-show, one time binding, ng-repeat with track by)
Opened this issue · 0 comments
Hi!
I would like to propose several performance tweaks to this directive!
First (ng-show vs ng-if):
I would suggest avoiding ng-if for showing/hiding icons.
For instance here: https://github.com/khan4019/tree-grid-directive/blob/master/src/tree-grid-directive.js#L13
<a ng-if=\"col.sortable\" ng-click=\"sortBy(col)\">{{col.displayName || col.field}}</a><span ng-if=\"!col.sortable\">{{col.displayName || col.field}}</span>
Here ng-show/ng-hide would be so much better, because
- it does not open a new scope (ng-if does)
- it does not remove/add elements from the DOM (ng-if does)
I believe that the only reason where ng-if is valid in that template would be when you combine it with compile
or any other heavy computation.
Second: Use Angular One Time Binding
I also think that you should make heavy use of Angulars one time binding syntax, using :: where it makes sense. One time binding will avoid creating a watcher.
For instance, look at this code:
" <td ng-repeat=\"col in colDefinitions\">\n" +
" <div ng-if=\"col.cellTemplate\" compile=\"col.cellTemplate\" cell-template-scope=\"col.cellTemplateScope\"></div>\n" +
" <div ng-if=\"!col.cellTemplate\">{{row.branch[col.field]}}</div>\n" +
" </td>\n" +
This will create a watcher for colDefinitions, and in addition to col.cellTemplate for each column.
In addition, this is within an ng-repeat of all rows, so you can multiply the amount of watchers...
As a counter example where you would NOT want one time binding to be active:
Showing the sorting icon depending on whether the column is sorted or not:
<i ng-show=\"col.sorted\" class=\"{{col.sortingIcon}} pull-right\"></i>
Third: Use ng-repeat with track by
ng-repeat makes heavy computation using a hash function. By using track-by with a unique identifier you can avoid this computation. for instance, ng-repeat on the column definitions could be tracked by their index, using
ng-repeat=\"col in ::colDefinitions track by $index\"
I'm waiting a little before I send a Pull Request, in case other people find reasons on why this or that should not be handled the way I proposed it. PLEASE DISCUSS!
For simplicity so people can try it out, here is my modified template:
function ($templateCache) {
$templateCache.put('template/treeGrid/treeGrid.html',
"<div class=\"table-responsive\">\n" +
" <table class=\"table tree-grid\">\n" +
" <thead>\n" +
" <tr>\n" +
" <th><a ng-show=\"expandingProperty.sortable\" ng-click=\"sortBy(expandingProperty)\">{{expandingProperty.displayName || expandingProperty.field || expandingProperty}}</a><span ng-hide=\"expandingProperty.sortable\">{{expandingProperty.displayName || expandingProperty.field || expandingProperty}}</span><i ng-show=\"expandingProperty.sorted\" class=\"{{expandingProperty.sortingIcon}} pull-right\"></i></th>\n" +
" <th ng-repeat=\"col in ::colDefinitions track by $index\"><a ng-show=\"::col.sortable\" ng-click=\"sortBy(col)\">{{col.displayName || col.field}}</a><span ng-hide=\"::col.sortable\">{{col.displayName || col.field}}</span><i ng-show=\"col.sorted\" class=\"{{col.sortingIcon}} pull-right\"></i></th>\n" +
" </tr>\n" +
" </thead>\n" +
" <tbody>\n" +
" <tr ng-repeat=\"row in tree_rows | searchFor:$parent.filterString:expandingProperty:colDefinitions track by row.branch.uid\"\n" +
" ng-class=\"'level-' + {{ row.level }} + (row.branch.selected ? ' active':'')\" class=\"tree-grid-row\">\n" +
" <td><a ng-click=\"user_clicks_branch(row.branch)\"><i ng-class=\"row.tree_icon\"\n" +
" ng-click=\"row.branch.expanded = !row.branch.expanded\"\n" +
" class=\"indented tree-icon\"></i></a><span ng-if=\"expandingProperty.cellTemplate\" class=\"indented tree-label\" " +
" ng-click=\"on_user_click(row.branch)\" compile=\"expandingProperty.cellTemplate\"></span>" +
" <span ng-if=\"!expandingProperty.cellTemplate\" class=\"indented tree-label\" ng-click=\"on_user_click(row.branch)\">\n" +
" {{row.branch[expandingProperty.field] || row.branch[expandingProperty]}}</span>\n" +
" </td>\n" +
" <td ng-repeat=\"col in ::colDefinitions track by $index\">\n" +
" <div ng-if=\"::col.cellTemplate\" compile=\"col.cellTemplate\" cell-template-scope=\"col.cellTemplateScope\"></div>\n" +
" <div ng-if=\"!::col.cellTemplate\">{{row.branch[col.field]}}</div>\n" +
" </td>\n" +
" </tr>\n" +
" </tbody>\n" +
" </table>\n" +
"</div>\n" +
"");
}