Skip to content

Commit b0972a2

Browse files
tboschtbosch
committed
fix($compile): update cloned elements if the template arrives after the cloning
If an element has a directive whose content is loaded using `templateUrl`, and the element is cloned using a linking function before the template arrives, the clone needs to be updated as well. This also updates `ngIf` and `ngRepeat` to keep the connection to the clone of a tranclude function, so that they know about the changes a directive with `templateUrl` does to the element in the future. Fixes to angular#4930.
1 parent 2dbb6f9 commit b0972a2

File tree

7 files changed

+138
-23
lines changed

7 files changed

+138
-23
lines changed

‎src/Angular.js

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,23 +1330,25 @@ function getter(obj, path, bindFnToScope) {
13301330
}
13311331

13321332
/**
1333-
* Return the siblings between `startNode` and `endNode`, inclusive
1334-
* @param {Object} object with `startNode` and `endNode` properties
1333+
* Return the DOM siblings between the first and last node in the given array.
1334+
* @param {Array} array like object
13351335
* @returns jQlite object containing the elements
13361336
*/
1337-
function getBlockElements(block) {
1338-
if (block.startNode === block.endNode) {
1339-
return jqLite(block.startNode);
1337+
function getBlockElements(nodes) {
1338+
var startNode = nodes[0],
1339+
endNode = nodes[nodes.length - 1];
1340+
if (startNode === endNode) {
1341+
return jqLite(startNode);
13401342
}
13411343

1342-
var element = block.startNode;
1344+
var element = startNode;
13431345
var elements = [element];
13441346

13451347
do {
13461348
element = element.nextSibling;
13471349
if (!element) break;
13481350
elements.push(element);
1349-
} while (element !== block.endNode);
1351+
} while (element !== endNode);
13501352

13511353
return jqLite(elements);
13521354
}

‎src/ng/compile.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -931,7 +931,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
931931
createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn)
932932
);
933933
} else {
934-
nodeLinkFn(childLinkFn, childScope, node, undefined, boundTranscludeFn);
934+
nodeLinkFn(childLinkFn, childScope, node, $rootElement, boundTranscludeFn);
935935
}
936936
} else if (childLinkFn) {
937937
childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn);

‎src/ng/directive/ngIf.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,12 @@ var ngIfDirective = ['$animate', function($animate) {
9494
if (!childScope) {
9595
childScope = $scope.$new();
9696
$transclude(childScope, function (clone) {
97+
clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ');
98+
// Note: We only need the first/last node of the cloned nodes.
99+
// However, we need to keep the reference to the jqlite wrapper as it might be changed later
100+
// by a directive with templateUrl when it's template arrives.
97101
block = {
98-
startNode: clone[0],
99-
endNode: clone[clone.length++] = document.createComment(' end ngIf: ' + $attr.ngIf + ' ')
102+
clone: clone
100103
};
101104
$animate.enter(clone, $element.parent(), $element);
102105
});
@@ -109,7 +112,7 @@ var ngIfDirective = ['$animate', function($animate) {
109112
}
110113

111114
if (block) {
112-
$animate.leave(getBlockElements(block));
115+
$animate.leave(getBlockElements(block.clone));
113116
block = null;
114117
}
115118
}

‎src/ng/directive/ngRepeat.js

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
301301
} else if (nextBlockMap.hasOwnProperty(trackById)) {
302302
// restore lastBlockMap
303303
forEach(nextBlockOrder, function(block) {
304-
if (block && block.startNode) lastBlockMap[block.id] = block;
304+
if (block && block.scope) lastBlockMap[block.id] = block;
305305
});
306306
// This is a duplicate and we need to throw an error
307307
throw ngRepeatMinErr('dupes', "Duplicates in a repeater are not allowed. Use 'track by' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}",
@@ -318,7 +318,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
318318
// lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn
319319
if (lastBlockMap.hasOwnProperty(key)) {
320320
block = lastBlockMap[key];
321-
elementsToRemove = getBlockElements(block);
321+
elementsToRemove = getBlockElements(block.clone);
322322
$animate.leave(elementsToRemove);
323323
forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; });
324324
block.scope.$destroy();
@@ -330,9 +330,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
330330
key = (collection === collectionKeys) ? index : collectionKeys[index];
331331
value = collection[key];
332332
block = nextBlockOrder[index];
333-
if (nextBlockOrder[index - 1]) previousNode = nextBlockOrder[index - 1].endNode;
333+
if (nextBlockOrder[index - 1]) previousNode = getBlockEnd(nextBlockOrder[index - 1]);
334334

335-
if (block.startNode) {
335+
if (block.scope) {
336336
// if we have already seen this object, then we need to reuse the
337337
// associated scope/element
338338
childScope = block.scope;
@@ -342,11 +342,11 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
342342
nextNode = nextNode.nextSibling;
343343
} while(nextNode && nextNode[NG_REMOVED]);
344344

345-
if (block.startNode != nextNode) {
345+
if (getBlockStart(block) != nextNode) {
346346
// existing item which got moved
347-
$animate.move(getBlockElements(block), null, jqLite(previousNode));
347+
$animate.move(getBlockElements(block.clone), null, jqLite(previousNode));
348348
}
349-
previousNode = block.endNode;
349+
previousNode = getBlockEnd(block);
350350
} else {
351351
// new item which we don't know about
352352
childScope = $scope.$new();
@@ -362,14 +362,16 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
362362
childScope.$odd = !(childScope.$even = (index&1) === 0);
363363
// jshint bitwise: true
364364

365-
if (!block.startNode) {
365+
if (!block.scope) {
366366
$transclude(childScope, function(clone) {
367367
clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');
368368
$animate.enter(clone, null, jqLite(previousNode));
369369
previousNode = clone;
370370
block.scope = childScope;
371-
block.startNode = previousNode && previousNode.endNode ? previousNode.endNode : clone[0];
372-
block.endNode = clone[clone.length - 1];
371+
// Note: We only need the first/last node of the cloned nodes.
372+
// However, we need to keep the reference to the jqlite wrapper as it might be changed later
373+
// by a directive with templateUrl when it's template arrives.
374+
block.clone = clone;
373375
nextBlockMap[block.id] = block;
374376
});
375377
}
@@ -378,5 +380,13 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
378380
});
379381
}
380382
};
383+
384+
function getBlockStart(block) {
385+
return block.clone[0];
386+
}
387+
388+
function getBlockEnd(block) {
389+
return block.clone[block.clone.length - 1];
390+
}
381391
}];
382392

‎test/ng/compileSpec.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,32 @@ describe('$compile', function() {
969969
});
970970
});
971971

972+
it('should resolve widgets after cloning in append mode without $templateCache', function() {
973+
module(function($exceptionHandlerProvider) {
974+
$exceptionHandlerProvider.mode('log');
975+
});
976+
inject(function($compile, $templateCache, $rootScope, $httpBackend, $browser,
977+
$exceptionHandler) {
978+
$httpBackend.expect('GET', 'cau.html').respond('<span>{{name}}</span>');
979+
$rootScope.name = 'Elvis';
980+
var template = $compile('<div class="cau"></div>');
981+
var e1;
982+
var e2;
983+
984+
e1 = template($rootScope.$new(), noop); // clone
985+
expect(e1.text()).toEqual('');
986+
987+
$httpBackend.flush();
988+
989+
e2 = template($rootScope.$new(), noop); // clone
990+
$rootScope.$digest();
991+
expect(e1.text()).toEqual('Elvis');
992+
expect(e2.text()).toEqual('Elvis');
993+
994+
dealoc(e1);
995+
dealoc(e2);
996+
});
997+
});
972998

973999
it('should resolve widgets after cloning in inline mode', function() {
9741000
module(function($exceptionHandlerProvider) {
@@ -1010,6 +1036,33 @@ describe('$compile', function() {
10101036
});
10111037
});
10121038

1039+
it('should resolve widgets after cloning in inline mode without $templateCache', function() {
1040+
module(function($exceptionHandlerProvider) {
1041+
$exceptionHandlerProvider.mode('log');
1042+
});
1043+
inject(function($compile, $templateCache, $rootScope, $httpBackend, $browser,
1044+
$exceptionHandler) {
1045+
$httpBackend.expect('GET', 'cau.html').respond('<span>{{name}}</span>');
1046+
$rootScope.name = 'Elvis';
1047+
var template = $compile('<div class="i-cau"></div>');
1048+
var e1;
1049+
var e2;
1050+
1051+
e1 = template($rootScope.$new(), noop); // clone
1052+
expect(e1.text()).toEqual('');
1053+
1054+
$httpBackend.flush();
1055+
1056+
e2 = template($rootScope.$new(), noop); // clone
1057+
$rootScope.$digest();
1058+
expect(e1.text()).toEqual('Elvis');
1059+
expect(e2.text()).toEqual('Elvis');
1060+
1061+
dealoc(e1);
1062+
dealoc(e2);
1063+
});
1064+
});
1065+
10131066

10141067
it('should be implicitly terminal and not compile placeholder content in append', inject(
10151068
function($compile, $templateCache, $rootScope, log) {

‎test/ng/directive/ngIfSpec.js

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
'use strict';
22

33
describe('ngIf', function () {
4-
var $scope, $compile, element;
4+
var $scope, $compile, element, $compileProvider;
55

6+
beforeEach(module(function(_$compileProvider_) {
7+
$compileProvider = _$compileProvider_;
8+
}));
69
beforeEach(inject(function ($rootScope, _$compile_) {
710
$scope = $rootScope.$new();
811
$compile = _$compile_;
@@ -146,6 +149,28 @@ describe('ngIf', function () {
146149
expect(element.children()[0].className).toContain('my-class');
147150
});
148151

152+
it('should work when combined with an ASYNC template that loads after the first digest', inject(function($httpBackend, $compile, $rootScope) {
153+
$compileProvider.directive('test', function() {
154+
return {
155+
templateUrl: 'test.html'
156+
};
157+
});
158+
$httpBackend.whenGET('test.html').respond('hello');
159+
element.append('<div ng-if="show" test></div>');
160+
$compile(element)($rootScope);
161+
$rootScope.show = true;
162+
$rootScope.$apply();
163+
expect(element.text()).toBe('');
164+
165+
$httpBackend.flush();
166+
expect(element.text()).toBe('hello');
167+
168+
$rootScope.show = false;
169+
$rootScope.$apply();
170+
// Note: there are still comments in element!
171+
expect(element.children().length).toBe(0);
172+
expect(element.text()).toBe('');
173+
}));
149174
});
150175

151176
describe('ngIf and transcludes', function() {

‎test/ng/directive/ngRepeatSpec.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,8 +749,30 @@ describe('ngRepeat', function() {
749749
expect(element.text()).toBe('123');
750750
}));
751751
}
752-
});
753752

753+
it('should work when combined with an ASYNC template that loads after the first digest', inject(function($httpBackend, $compile, $rootScope) {
754+
$compileProvider.directive('test', function() {
755+
return {
756+
templateUrl: 'test.html'
757+
};
758+
});
759+
$httpBackend.whenGET('test.html').respond('hello');
760+
element = jqLite('<div><div ng-repeat="i in items" test></div></div>');
761+
$compile(element)($rootScope);
762+
$rootScope.items = [1];
763+
$rootScope.$apply();
764+
expect(element.text()).toBe('');
765+
766+
$httpBackend.flush();
767+
expect(element.text()).toBe('hello');
768+
769+
$rootScope.items = [];
770+
$rootScope.$apply();
771+
// Note: there are still comments in element!
772+
expect(element.children().length).toBe(0);
773+
expect(element.text()).toBe('');
774+
}));
775+
});
754776

755777
it('should add separator comments after each item', inject(function ($compile, $rootScope) {
756778
var check = function () {

0 commit comments

Comments
 (0)