fix(ngRepeat): prevent initial duplicates

angular · Apr 12, 2013 · a0bc71e · a0bc71e
1 parent a491ea3
commit a0bc71e
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 10 deletions.
diff --git a/docs/content/cookbook/mvc.ngdoc b/docs/content/cookbook/mvc.ngdoc
@@ -85,8 +85,8 @@ view.
     Next Player: {{nextMove}}
     <div class="winner" ng-show="winner">Player {{winner}} has won!</div>
       <table class="board">
-        <tr ng-repeat="row in board" style="height:15px;">
-          <td ng-repeat="cell in row" ng-style="cellStyle"
+        <tr ng-repeat="row in board track by $index" style="height:15px;">
+          <td ng-repeat="cell in row track by $index" ng-style="cellStyle"
               ng-click="dropPiece($parent.$index, $index)">{{cell}}</td>
         </tr>
       </table>

diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js
@@ -153,7 +153,7 @@ var ngRepeatDirective = ['$parse', '$animator', function($parse, $animator) {
         var animate = $animator($scope, $attr);
         var expression = $attr.ngRepeat;
         var match = expression.match(/^\s*(.+)\s+in\s+(.*?)\s*(\s+track\s+by\s+(.+)\s*)?$/),
-          trackByExp, hashExpFn, trackByIdFn, lhs, rhs, valueIdentifier, keyIdentifier,
+          trackByExp, trackByExpGetter, trackByIdFn, lhs, rhs, valueIdentifier, keyIdentifier,
           hashFnLocals = {$id: hashKey};
 
         if (!match) {
@@ -166,13 +166,13 @@ var ngRepeatDirective = ['$parse', '$animator', function($parse, $animator) {
         trackByExp = match[4];
 
         if (trackByExp) {
-          hashExpFn = $parse(trackByExp);
+          trackByExpGetter = $parse(trackByExp);
           trackByIdFn = function(key, value, index) {
             // assign key, value, and $index to the locals so that they can be used in hash functions
             if (keyIdentifier) hashFnLocals[keyIdentifier] = key;
             hashFnLocals[valueIdentifier] = value;
             hashFnLocals.$index = index;
-            return hashExpFn($scope, hashFnLocals);
+            return trackByExpGetter($scope, hashFnLocals);
           };
         } else {
           trackByIdFn = function(key, value) {
@@ -233,7 +233,8 @@ var ngRepeatDirective = ['$parse', '$animator', function($parse, $animator) {
            key = (collection === collectionKeys) ? index : collectionKeys[index];
            value = collection[key];
            trackById = trackByIdFn(key, value, index);
-           if((block = lastBlockMap[trackById])) {
+           if(lastBlockMap.hasOwnProperty(trackById)) {
+             block = lastBlockMap[trackById]
              delete lastBlockMap[trackById];
              nextBlockMap[trackById] = block;
              nextBlockOrder[index] = block;
@@ -243,10 +244,12 @@ var ngRepeatDirective = ['$parse', '$animator', function($parse, $animator) {
                if (block && block.element) lastBlockMap[block.id] = block;
              });
              // This is a duplicate and we need to throw an error
-             throw new Error('Duplicates in a repeater are not allowed. Repeater: ' + expression);
+             throw new Error('Duplicates in a repeater are not allowed. Repeater: ' + expression +
+                 ' key: ' + trackById);
            } else {
              // new never before seen block
              nextBlockOrder[index] = { id: trackById };
+             nextBlockMap[trackById] = false;
            }
          }
 

diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js
@@ -391,7 +391,7 @@ describe('ngRepeat', function() {
 
 
   it('should iterate over non-existent elements of a sparse array', function() {
-    element = $compile('<ul><li ng-repeat="item in array">{{item}}|</li></ul>')(scope);
+    element = $compile('<ul><li ng-repeat="item in array track by $index">{{item}}|</li></ul>')(scope);
     scope.array = ['a', 'b'];
     scope.array[4] = 'c';
     scope.array[6] = 'd';
@@ -457,11 +457,31 @@ describe('ngRepeat', function() {
     });
 
 
-    it('should throw error on duplicates and recover', function() {
+    it('should throw error on adding existing duplicates and recover', function() {
       scope.items = [a, a, a];
       scope.$digest();
       expect($exceptionHandler.errors.shift().message).
-          toEqual('Duplicates in a repeater are not allowed. Repeater: item in items');
+          toEqual('Duplicates in a repeater are not allowed. Repeater: item in items key: object:003');
+
+      // recover
+      scope.items = [a];
+      scope.$digest();
+      var newElements = element.find('li');
+      expect(newElements.length).toEqual(1);
+      expect(newElements[0]).toEqual(lis[0]);
+
+      scope.items = [];
+      scope.$digest();
+      var newElements = element.find('li');
+      expect(newElements.length).toEqual(0);
+    });
+
+
+    it('should throw error on new duplicates and recover', function() {
+      scope.items = [d, d, d];
+      scope.$digest();
+      expect($exceptionHandler.errors.shift().message).
+          toEqual('Duplicates in a repeater are not allowed. Repeater: item in items key: object:009');
 
       // recover
       scope.items = [a];