Skip to content

Commit

Permalink
fix(uiGridCell): Use promises for tmpl processing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
c0bra committed Oct 7, 2014
1 parent 3d4e857 commit 9fb29cc
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 10 deletions.
3 changes: 3 additions & 0 deletions misc/demo/cellTemplate.html
@@ -0,0 +1,3 @@
<div class="ui-grid-cell-contents">
Testing $http template: {{COL_FIELD CUSTOM_FILTERS}}
</div>
14 changes: 11 additions & 3 deletions misc/demo/grid-directive.html
Expand Up @@ -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) {
Expand Down
13 changes: 11 additions & 2 deletions src/js/core/directives/ui-grid-cell.js
Expand Up @@ -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);
}
Expand Down
4 changes: 4 additions & 0 deletions src/js/core/factories/Grid.js
Expand Up @@ -429,6 +429,10 @@ angular.module('ui.grid')

var compiledElementFn = $compile(html);
col.compiledElementFn = compiledElementFn;

if (col.compiledElementFnDefer) {
col.compiledElementFnDefer.resolve(col.compiledElementFn);
}
});
};

Expand Down
12 changes: 12 additions & 0 deletions src/js/core/factories/GridColumn.js
Expand Up @@ -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;
}]);

Expand Down
8 changes: 6 additions & 2 deletions src/js/core/services/gridClassFactory.js
Expand Up @@ -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 : "");
Expand All @@ -136,7 +138,6 @@
})
);


templateGetPromises.push(gridUtil.getTemplate(colDef.headerCellTemplate)
.then(
function (template) {
Expand All @@ -147,6 +148,9 @@
})
);

// Create a promise for the compiled element function
col.compiledElementFnDefer = $q.defer();

return $q.all(templateGetPromises);
}

Expand Down
116 changes: 113 additions & 3 deletions 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' }
Expand Down Expand Up @@ -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);
});
});
});
});

0 comments on commit 9fb29cc

Please sign in to comment.