fix(uiGridCell): Use promises for tmpl processing

Issue #1712 demonstrates a race condition where a column's compiledElementFn is attempted to be called before the cellTemplate is present, e.g. in the case of a template fetched over the network. This causes the function to not be present and an exception to be thrown. This changes fixes this by added a getCompiledElementFn() method to GridColumn, which returns a promise that is created in the defaultColumnBuilder(). It gets resolved in Grid.preCompileCellTemplates(). uiGridCell now uses this promise if the function is not present at pre-link time. Added tests for getCompiledElementFn() and getCellTemplate() Fixes #1712
angular-ui · Oct 7, 2014 · 9fb29cc · 9fb29cc
1 parent 3d4e857
commit 9fb29cc
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 10 deletions.
diff --git a/misc/demo/cellTemplate.html b/misc/demo/cellTemplate.html
@@ -0,0 +1,3 @@
+<div class="ui-grid-cell-contents">
+    Testing $http template: {{COL_FIELD CUSTOM_FILTERS}}
+</div>
diff --git a/misc/demo/grid-directive.html b/misc/demo/grid-directive.html
@@ -46,12 +46,20 @@ <h2>Grid</h2>
 
       <script>
 
-      var app = angular.module('test', ['ui.grid', 'ui.grid.selection']);
+      var app = angular.module('test', ['ui.grid', 'ui.grid.selection', 'ui.grid.expandable']);
 
       app.controller('Main', function($scope, $http) {
           // $scope.gridOptions = { data: 'data' };
-          $scope.gridOptions = {};
-          // $scope.gridOptions.columnDefs = [ {name:"First Name", field:"firstName"} ];
+          $scope.gridOptions = {
+            enableFiltering: true
+          };
+          $scope.gridOptions.columnDefs = [
+            {
+              name : "Name",
+              field: "name",
+              cellTemplate: '/misc/demo/cellTemplate.html'
+            }
+          ];
 
           $http.get('/misc/site/data/100.json')
             .success(function (data) {

diff --git a/src/js/core/directives/ui-grid-cell.js b/src/js/core/directives/ui-grid-cell.js
@@ -21,11 +21,20 @@ angular.module('ui.grid').directive('uiGridCell', ['$compile', '$log', '$parse',
           // No controller, compile the element manually (for unit tests)
           else {
             if ( uiGridCtrl && !$scope.col.compiledElementFn ){
-              $log.error('Render has been called before precompile.  Please log a ui-grid issue');  
-            } else {
+              // $log.error('Render has been called before precompile.  Please log a ui-grid issue');  
+
+              $scope.col.getCompiledElementFn()
+                .then(function (compiledElementFn) {
+                  compiledElementFn($scope, function(clonedElement, scope) {
+                    $elm.append(clonedElement);
+                  });
+                });
+            }
+            else {
               var html = $scope.col.cellTemplate
                 .replace(uiGridConstants.MODEL_COL_FIELD, 'row.entity.' + gridUtil.preEval($scope.col.field))
                 .replace(uiGridConstants.COL_FIELD, 'grid.getCellValue(row, col)');
+
               var cellElement = $compile(html)($scope);
               $elm.append(cellElement);
             }

diff --git a/src/js/core/factories/Grid.js b/src/js/core/factories/Grid.js
@@ -429,6 +429,10 @@ angular.module('ui.grid')
 
       var compiledElementFn = $compile(html);
       col.compiledElementFn = compiledElementFn;
+
+      if (col.compiledElementFnDefer) {
+        col.compiledElementFnDefer.resolve(col.compiledElementFn);
+      }
     });
   };
 

diff --git a/src/js/core/factories/GridColumn.js b/src/js/core/factories/GridColumn.js
@@ -661,6 +661,18 @@ angular.module('ui.grid')
       }
     };
 
+    GridColumn.prototype.getCellTemplate = function () {
+      var self = this;
+
+      return self.cellTemplatePromise;
+    };
+
+    GridColumn.prototype.getCompiledElementFn = function () {
+      var self = this;
+
+      return self.compiledElementFnDefer.promise;
+    };
+
     return GridColumn;
   }]);
 

diff --git a/src/js/core/services/gridClassFactory.js b/src/js/core/services/gridClassFactory.js
@@ -124,9 +124,11 @@
            */
           if (!colDef.cellTemplate) {
             colDef.cellTemplate = 'ui-grid/uiGridCell';
+            col.cellTemplatePromise = gridUtil.getTemplate(colDef.cellTemplate);
           }
 
-          templateGetPromises.push(gridUtil.getTemplate(colDef.cellTemplate)
+          col.cellTemplatePromise = gridUtil.getTemplate(colDef.cellTemplate);
+          templateGetPromises.push(col.cellTemplatePromise
             .then(
               function (template) {
                 col.cellTemplate = template.replace(uiGridConstants.CUSTOM_FILTERS, col.cellFilter ? "|" + col.cellFilter : "");
@@ -136,7 +138,6 @@
               })
           );
 
-
           templateGetPromises.push(gridUtil.getTemplate(colDef.headerCellTemplate)
               .then(
               function (template) {
@@ -147,6 +148,9 @@
               })
           );
 
+          // Create a promise for the compiled element function
+          col.compiledElementFnDefer = $q.defer();
+
           return $q.all(templateGetPromises);
         }
 

diff --git a/test/unit/core/factories/GridColumn.spec.js b/test/unit/core/factories/GridColumn.spec.js
@@ -1,21 +1,22 @@
 describe('GridColumn factory', function () {
-  var $q, $scope, cols, grid, gridCol, Grid, GridColumn, gridClassFactory, GridRenderContainer, uiGridConstants;
+  var $q, $scope, cols, grid, gridCol, Grid, GridColumn, gridClassFactory, GridRenderContainer, uiGridConstants, $httpBackend;
 
   beforeEach(module('ui.grid'));
 
   function buildCols() {
-    grid.buildColumns();
+    return grid.buildColumns();
   }
 
 
-  beforeEach(inject(function (_$q_, _$rootScope_, _Grid_, _GridColumn_, _gridClassFactory_, _GridRenderContainer_, _uiGridConstants_) {
+  beforeEach(inject(function (_$q_, _$rootScope_, _Grid_, _GridColumn_, _gridClassFactory_, _GridRenderContainer_, _uiGridConstants_, _$httpBackend_) {
     $q = _$q_;
     $scope = _$rootScope_;
     Grid = _Grid_;
     GridColumn = _GridColumn_;
     gridClassFactory = _gridClassFactory_;
     GridRenderContainer = _GridRenderContainer_;
     uiGridConstants = _uiGridConstants_;
+    $httpBackend = _$httpBackend_;
 
     cols = [
       { field: 'firstName' }
@@ -220,4 +221,113 @@ describe('GridColumn factory', function () {
     });
   });
 
+  describe('getCompiledElementFn()', function () {
+    var col;
+
+    beforeEach(function () {
+      col = grid.columns[0];
+    });
+
+    it('should return a promise', function () {
+      expect(col.getCompiledElementFn().hasOwnProperty('then')).toBe(true);
+    });
+
+    it('should return a promise that is resolved when the cellTemplate is compiled', function () {
+      var resolved = false;
+
+      runs(function () {
+        buildCols().then(function () {
+          grid.preCompileCellTemplates();
+        });
+      });
+
+      runs(function () {
+        col.getCompiledElementFn().then(function () {
+          resolved = true;
+        });
+
+        $scope.$digest();
+      });
+
+      // $scope.$digest();
+
+      runs(function () {
+        expect(resolved).toBe(true);
+      });
+    });
+
+    it('should return a promise that is resolved when a URL-based cellTemplate is available', function () {
+      var resolved = false;
+
+      var url = 'http://www.a-really-fake-url.com/template.html';
+      cols[0].cellTemplate = url;
+
+      $httpBackend.when('GET', url).respond('<div>foo</div>');
+
+      runs(function () {
+        buildCols().then(function () {
+          grid.preCompileCellTemplates();
+        });
+
+        col.getCompiledElementFn().then(function () {
+          resolved = true;
+        });
+
+        expect(resolved).toBe(false);
+
+        $httpBackend.flush();
+      });
+
+      runs(function () {
+        $scope.$digest();
+      });
+
+      runs(function () {
+        expect(resolved).toBe(true);
+      });
+    });
+  });
+
+  describe('getCellTemplate()', function () {
+    var col;
+
+    beforeEach(function () {
+      col = grid.columns[0];
+    });
+
+    it('should return a promise', function () {
+      expect(col.getCellTemplate().hasOwnProperty('then')).toBe(true);
+    });
+
+    it('should return a promise that is resolved when a URL-based cellTemplate is available', function () {
+      var resolved = false;
+
+      var url = 'http://www.a-really-fake-url.com/template.html';
+      cols[0].cellTemplate = url;
+
+      $httpBackend.when('GET', url).respond('<div>foo</div>');
+
+      runs(function () {
+        buildCols().then(function () {
+          grid.preCompileCellTemplates();
+        });
+
+        col.getCellTemplate().then(function () {
+          resolved = true;
+        });
+
+        expect(resolved).toBe(false);
+
+        $httpBackend.flush();
+      });
+
+      runs(function () {
+        $scope.$digest();
+      });
+
+      runs(function () {
+        expect(resolved).toBe(true);
+      });
+    });
+  });
 });