From 624fd80854b912ab5fe6b282264f88f42ab594b1 Mon Sep 17 00:00:00 2001
From: jankuca <jan@jankuca.com>
Date: Mon, 23 Sep 2013 11:24:42 -0700
Subject: [PATCH] feat(ngRepeat): use block separator comments

Closes #3104
---
 src/ng/directive/ngRepeat.js      |  21 +++++-
 test/BinderSpec.js                |  27 ++++++-
 test/ng/directive/ngClassSpec.js  |  10 +--
 test/ng/directive/ngRepeatSpec.js | 121 ++++++++++++++++++++++++++----
 4 files changed, 151 insertions(+), 28 deletions(-)

diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js
index 50be2a618e68..d31f2d201bba 100644
--- a/src/ng/directive/ngRepeat.js
+++ b/src/ng/directive/ngRepeat.js
@@ -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();
             }
           }
@@ -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
@@ -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;
diff --git a/test/BinderSpec.js b/test/BinderSpec.js
index 3c204b64c48b..b553c68dcfd0 100644
--- a/test/BinderSpec.js
+++ b/test/BinderSpec.js
@@ -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'});
@@ -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();
@@ -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();
@@ -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>');
   }));
 
@@ -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() {
@@ -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>');
   }));
 
@@ -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>');
   }));
 
@@ -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>');
   }));
 
diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js
index f0989f4a9531..a788e45255b5 100644
--- a/test/ng/directive/ngClassSpec.js
+++ b/test/ng/directive/ngClassSpec.js
@@ -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();
@@ -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();
@@ -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();
@@ -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();
@@ -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();
diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js
index fc41bc6d71eb..3e230a5862ee 100644
--- a/test/ng/directive/ngRepeatSpec.js
+++ b/test/ng/directive/ngRepeatSpec.js
@@ -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>'
       );
     }));
@@ -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>'
       );
     }));
@@ -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
+      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;
 
@@ -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;