Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

feat(ngRepeat): use block separator comments #4157

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/ng/directive/ngRepeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,21 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
for (key in lastBlockMap) {
if (lastBlockMap.hasOwnProperty(key)) {
block = lastBlockMap[key];
$animate.leave(block.elements);
forEach(block.elements, function(element) { element[NG_REMOVED] = true});

var elementsToRemove = [];
var elementToRemove = block.startNode;
elementsToRemove.push(elementToRemove);

if (block.startNode !== block.endNode) {
do {
elementToRemove = elementToRemove.nextSibling;
if (!elementToRemove) break;
elementsToRemove.push(elementToRemove);
} while (elementToRemove !== block.endNode);
}

$animate.leave(angular.element(elementsToRemove));
forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; });
block.scope.$destroy();
}
}
Expand All @@ -338,6 +351,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
key = (collection === collectionKeys) ? index : collectionKeys[index];
value = collection[key];
block = nextBlockOrder[index];
if (nextBlockOrder[index - 1]) previousNode = nextBlockOrder[index - 1].endNode;

if (block.startNode) {
// if we have already seen this object, then we need to reuse the
Expand Down Expand Up @@ -371,10 +385,11 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {

if (!block.startNode) {
linker(childScope, function(clone) {
clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');
$animate.enter(clone, null, jqLite(previousNode));
previousNode = clone;
block.scope = childScope;
block.startNode = clone[0];
block.startNode = previousNode && previousNode.endNode ? previousNode.endNode : clone[0];
block.elements = clone;
block.endNode = clone[clone.length - 1];
nextBlockMap[block.id] = block;
Expand Down
27 changes: 23 additions & 4 deletions test/BinderSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ describe('Binder', function() {
'<ul>' +
'<!-- ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">A</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">B</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'</ul>');

items.unshift({a: 'C'});
Expand All @@ -105,8 +107,11 @@ describe('Binder', function() {
'<ul>' +
'<!-- ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">C</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">A</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">B</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'</ul>');

items.shift();
Expand All @@ -115,7 +120,9 @@ describe('Binder', function() {
'<ul>' +
'<!-- ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">A</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'<li ng-bind="item.a" ng-repeat="item in model.items">B</li>' +
'<!-- end ngRepeat: item in model.items -->' +
'</ul>');

items.shift();
Expand All @@ -134,6 +141,7 @@ describe('Binder', function() {
'<ul>' +
'<!-- ngRepeat: item in model.items -->' +
'<li ng-repeat="item in model.items"><span ng-bind="item.a">A</span></li>' +
'<!-- end ngRepeat: item in model.items -->' +
'</ul>');
}));

Expand All @@ -148,15 +156,15 @@ describe('Binder', function() {
$rootScope.items = items;

$rootScope.$apply();
expect(element[0].childNodes.length - 1).toEqual(0);
expect(element[0].childNodes.length).toEqual(1);

items.name = 'misko';
$rootScope.$apply();
expect(element[0].childNodes.length - 1).toEqual(1);
expect(element[0].childNodes.length).toEqual(3);

delete items.name;
$rootScope.$apply();
expect(element[0].childNodes.length - 1).toEqual(0);
expect(element[0].childNodes.length).toEqual(1);
}));

it('IfTextBindingThrowsErrorDecorateTheSpan', function() {
Expand Down Expand Up @@ -223,13 +231,19 @@ describe('Binder', function() {
'<div name="a" ng-repeat="m in model">'+
'<!-- ngRepeat: i in m.item -->' +
'<ul name="a1" ng-repeat="i in m.item"></ul>'+
'<!-- end ngRepeat: i in m.item -->' +
'<ul name="a2" ng-repeat="i in m.item"></ul>'+
'<!-- end ngRepeat: i in m.item -->' +
'</div>'+
'<!-- end ngRepeat: m in model -->' +
'<div name="b" ng-repeat="m in model">'+
'<!-- ngRepeat: i in m.item -->' +
'<ul name="b1" ng-repeat="i in m.item"></ul>'+
'<!-- end ngRepeat: i in m.item -->' +
'<ul name="b2" ng-repeat="i in m.item"></ul>'+
'<!-- end ngRepeat: i in m.item -->' +
'</div>' +
'<!-- end ngRepeat: m in model -->' +
'</div>');
}));

Expand Down Expand Up @@ -306,15 +320,18 @@ describe('Binder', function() {
'<div ng-repeat="i in [0,1]" ng-class-even="\'e\'" ng-class-odd="\'o\'"></div>' +
'</div>')($rootScope);
$rootScope.$apply();

var d1 = jqLite(element[0].childNodes[1]);
var d2 = jqLite(element[0].childNodes[2]);
var d2 = jqLite(element[0].childNodes[3]);
expect(d1.hasClass('o')).toBeTruthy();
expect(d2.hasClass('e')).toBeTruthy();
expect(sortedHtml(element)).toBe(
'<div>' +
'<!-- ngRepeat: i in [0,1] -->' +
'<div class="o" ng-class-even="\'e\'" ng-class-odd="\'o\'" ng-repeat="i in [0,1]"></div>' +
'<!-- end ngRepeat: i in [0,1] -->' +
'<div class="e" ng-class-even="\'e\'" ng-class-odd="\'o\'" ng-repeat="i in [0,1]"></div>' +
'<!-- end ngRepeat: i in [0,1] -->' +
'</div>');
}));

Expand Down Expand Up @@ -420,7 +437,9 @@ describe('Binder', function() {
'<ul>' +
'<!-- ngRepeat: (k,v) in {a:0,b:1} -->' +
'<li ng-bind=\"k + v\" ng-repeat="(k,v) in {a:0,b:1}">a0</li>' +
'<!-- end ngRepeat: (k,v) in {a:0,b:1} -->' +
'<li ng-bind=\"k + v\" ng-repeat="(k,v) in {a:0,b:1}">b1</li>' +
'<!-- end ngRepeat: (k,v) in {a:0,b:1} -->' +
'</ul>');
}));

Expand Down
10 changes: 5 additions & 5 deletions test/ng/directive/ngClassSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe('ngClass', function() {
element = $compile('<ul><li ng-repeat="i in [0,1]" class="existing" ng-class-odd="\'odd\'" ng-class-even="\'even\'"></li><ul>')($rootScope);
$rootScope.$digest();
var e1 = jqLite(element[0].childNodes[1]);
var e2 = jqLite(element[0].childNodes[2]);
var e2 = jqLite(element[0].childNodes[3]);
expect(e1.hasClass('existing')).toBeTruthy();
expect(e1.hasClass('odd')).toBeTruthy();
expect(e2.hasClass('existing')).toBeTruthy();
Expand All @@ -181,7 +181,7 @@ describe('ngClass', function() {
'<ul>')($rootScope);
$rootScope.$apply();
var e1 = jqLite(element[0].childNodes[1]);
var e2 = jqLite(element[0].childNodes[2]);
var e2 = jqLite(element[0].childNodes[3]);

expect(e1.hasClass('plainClass')).toBeTruthy();
expect(e1.hasClass('odd')).toBeTruthy();
Expand All @@ -199,7 +199,7 @@ describe('ngClass', function() {
'<ul>')($rootScope);
$rootScope.$apply();
var e1 = jqLite(element[0].childNodes[1]);
var e2 = jqLite(element[0].childNodes[2]);
var e2 = jqLite(element[0].childNodes[3]);

expect(e1.hasClass('A')).toBeTruthy();
expect(e1.hasClass('B')).toBeTruthy();
Expand Down Expand Up @@ -273,7 +273,7 @@ describe('ngClass', function() {
$rootScope.$digest();

var e1 = jqLite(element[0].childNodes[1]);
var e2 = jqLite(element[0].childNodes[2]);
var e2 = jqLite(element[0].childNodes[3]);

expect(e1.hasClass('odd')).toBeTruthy();
expect(e1.hasClass('even')).toBeFalsy();
Expand All @@ -295,7 +295,7 @@ describe('ngClass', function() {
$rootScope.$digest();

var e1 = jqLite(element[0].childNodes[1]);
var e2 = jqLite(element[0].childNodes[2]);
var e2 = jqLite(element[0].childNodes[3]);

expect(e1.hasClass('odd')).toBeTruthy();
expect(e1.hasClass('even')).toBeFalsy();
Expand Down
121 changes: 105 additions & 16 deletions test/ng/directive/ngRepeatSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,9 @@ describe('ngRepeat', function() {
'<div>' +
'<!-- ngRepeat: i in items -->' +
'<div ng-repeat="i in items" rr="">1|</div>' +
'<!-- end ngRepeat: i in items -->' +
'<div ng-repeat="i in items" rr="">2|</div>' +
'<!-- end ngRepeat: i in items -->' +
'</div>'
);
}));
Expand Down Expand Up @@ -643,7 +645,9 @@ describe('ngRepeat', function() {
'<div>' +
'<!-- ngRepeat: i in items -->' +
'<div ng-repeat="i in items" rr="">1|</div>' +
'<!-- end ngRepeat: i in items -->' +
'<div ng-repeat="i in items" rr="">2|</div>' +
'<!-- end ngRepeat: i in items -->' +
'</div>'
);
}));
Expand Down Expand Up @@ -740,6 +744,66 @@ describe('ngRepeat', function() {
});


it('should add separator comments after each item', inject(function ($compile, $rootScope) {
var check = function () {
var children = element.find('div');
expect(children.length).toBe(3);

// Note: COMMENT_NODE === 8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to create a DSL, toBeComment()

expect(children[0].nextSibling.nodeType).toBe(8);
expect(children[0].nextSibling.nodeValue).toBe(' end ngRepeat: val in values ');
expect(children[1].nextSibling.nodeType).toBe(8);
expect(children[1].nextSibling.nodeValue).toBe(' end ngRepeat: val in values ');
expect(children[2].nextSibling.nodeType).toBe(8);
expect(children[2].nextSibling.nodeValue).toBe(' end ngRepeat: val in values ');
}

$rootScope.values = [1, 2, 3];

element = $compile(
'<div>' +
'<div ng-repeat="val in values">val:{{val}};</div>' +
'</div>'
)($rootScope);

$rootScope.$digest();
check();

$rootScope.values.shift();
$rootScope.values.push(4);
$rootScope.$digest();
check();
}));


it('should remove whole block even if the number of elements inside it changes', inject(
function ($compile, $rootScope) {

$rootScope.values = [1, 2, 3];

element = $compile(
'<div>' +
'<div ng-repeat-start="val in values"></div>' +
'<span>{{val}}</span>' +
'<p ng-repeat-end></p>' +
'</div>'
)($rootScope);

$rootScope.$digest();

var ends = element.find('p');
expect(ends.length).toBe(3);

// insert an extra element inside the second block
var extra = angular.element('<strong></strong>')[0];
element[0].insertBefore(extra, ends[1]);

$rootScope.values.splice(1, 1);
$rootScope.$digest();
expect(element.find('strong').length).toBe(0);
}));


describe('stability', function() {
var a, b, c, d, lis;

Expand Down Expand Up @@ -843,28 +907,53 @@ describe('ngRepeat', function() {
});
});

it('should grow multi-node repeater', inject(function($compile, $rootScope) {
$rootScope.show = false;
$rootScope.books = [
{title:'T1', description: 'D1'},
{title:'T2', description: 'D2'}
];
element = $compile(

describe('ngRepeatStart', function () {
it('should grow multi-node repeater', inject(function($compile, $rootScope) {
$rootScope.show = false;
$rootScope.books = [
{title:'T1', description: 'D1'},
{title:'T2', description: 'D2'}
];
element = $compile(
'<div>' +
'<dt ng-repeat-start="book in books">{{book.title}}:</dt>' +
'<dd ng-repeat-end>{{book.description}};</dd>' +
'</div>')($rootScope);

$rootScope.$digest();
expect(element.text()).toEqual('T1:D1;T2:D2;');
$rootScope.books.push({title:'T3', description: 'D3'});
$rootScope.$digest();
expect(element.text()).toEqual('T1:D1;T2:D2;T3:D3;');
}));


it('should not clobber ng-if when updating collection', inject(function ($compile, $rootScope) {
$rootScope.values = [1, 2, 3];
$rootScope.showMe = true;

element = $compile(
'<div>' +
'<dt ng-repeat-start="book in books">{{book.title}}:</dt>' +
'<dd ng-repeat-end>{{book.description}};</dd>' +
'</div>')($rootScope);
'<div ng-repeat-start="val in values">val:{{val}};</div>' +
'<div ng-if="showMe" ng-repeat-end>if:{{val}};</div>' +
'</div>'
)($rootScope);

$rootScope.$digest();
expect(element.text()).toEqual('T1:D1;T2:D2;');
$rootScope.books.push({title:'T3', description: 'D3'});
$rootScope.$digest();
expect(element.text()).toEqual('T1:D1;T2:D2;T3:D3;');
}));
$rootScope.$digest();
expect(element.find('div').length).toBe(6);

$rootScope.values.shift();
$rootScope.values.push(4);

$rootScope.$digest();
expect(element.find('div').length).toBe(6);
expect(element.text()).not.toContain('if:1;');
}));
});
});


describe('ngRepeat animations', function() {
var body, element, $rootElement;

Expand Down