refactor(ngRepeat): add tests and simplify code

This commits adds test for angular#933 and simplifies the implementation of the fix

Closes angular#933

src/apis.js

@@ -98,12 +98,12 @@ HashQueueMap.prototype = {
       }
     }
   },
-  
+
   /**
    * return the first item without deleting it
    */
   peek: function(key) {
-    var array = this[key = hashKey(key)];
+    var array = this[hashKey(key)];
     if (array) {
     return array[0];
     }

src/ng/directive/ngRepeat.js

@@ -88,7 +88,7 @@ var ngRepeatDirective = ngDirective({
       // We need an array of these objects since the same object can be returned from the iterator.
       // We expect this to be a rare case.
       var lastOrder = new HashQueueMap();
-      var indexValues = [];
+
       scope.$watch(function ngRepeatWatch(scope){
         var index, length,
             collection = scope.$eval(rhs),
@@ -119,18 +119,14 @@ var ngRepeatDirective = ngDirective({
           key = (collection === array) ? index : array[index];
           value = collection[key];
 
-          // if collection is array and value is object, it can be shifted to allow for position change
-          // if collection is array and value is not object, need to first check whether index is same to
-          // avoid shifting wrong value
-          // if collection is not array, need to always check index to avoid shifting wrong value
-          if (lastOrder.peek(value)) {
-            last = collection === array ?
-              ((isObject(value)) ? lastOrder.shift(value) :
-                (index === lastOrder.peek(value).index ? lastOrder.shift(value) : undefined)) :
-              (index === lastOrder.peek(value).index ? lastOrder.shift(value) : undefined);
-          } else {
-            last = undefined;
-          }
+          // if value is object, it can be shifted to allow for position change
+          // if is not object, need to first check whether index is same to avoid shifting wrong val
+          last = isObject(value)
+              ? lastOrder.shift(value)
+              : (last = lastOrder.peek(value)) && (index === last.index)
+                  ? lastOrder.shift(value)
+                  : undefined;
+
 
           if (last) {
             // if we have already seen this object, then we need to reuse the
@@ -151,12 +147,6 @@ var ngRepeatDirective = ngDirective({
               cursor = last.element;
             }
           } else {
-            if (indexValues.hasOwnProperty(index) && collection !== array) {
-              var preValue = indexValues[index];
-              var v = lastOrder.shift(preValue);
-              v.element.remove();
-              v.scope.$destroy();
-            }
             // new item which we don't know about
             childScope = scope.$new();
           }
@@ -178,16 +168,10 @@ var ngRepeatDirective = ngDirective({
                   index: index
                 };
               nextOrder.push(value, last);
-              indexValues[index] = value;
             });
           }
         }
 
-        var i, l;
-        for (i = 0, l = indexValues.length - length; i < l; i++) {
-          indexValues.pop();
-        }
-
         //shrink children
         for (key in lastOrder) {
           if (lastOrder.hasOwnProperty(key)) {

test/ng/directive/ngRepeatSpec.js

@@ -37,10 +37,10 @@ describe('ngRepeat', function() {
   }));
 
 
-  it('should ngRepeat over array of primitive correctly', inject(function($rootScope, $compile) {
+  it('should ngRepeat over array of primitives', inject(function($rootScope, $compile) {
     element = $compile(
       '<ul>' +
-        '<li ng-repeat="item in items" ng-init="suffix = \';\'" ng-bind="item + suffix"></li>' +
+        '<li ng-repeat="item in items">{{item}};</li>' +
       '</ul>')($rootScope);
 
     Array.prototype.extraProperty = "should be ignored";
@@ -91,13 +91,13 @@ describe('ngRepeat', function() {
     $rootScope.$digest();
     expect(element.find('li').length).toEqual(1);
     expect(element.text()).toEqual('test;');
-	
+
     $rootScope.items = ['same', 'value'];
     $rootScope.$digest();
     expect(element.find('li').length).toEqual(2);
     expect(element.text()).toEqual('same;value;');
-	
-	// number
+
+    // number
     $rootScope.items = [12, 12, 12];
     $rootScope.$digest();
     expect(element.find('li').length).toEqual(3);
@@ -130,36 +130,48 @@ describe('ngRepeat', function() {
     expect(element.text()).toEqual('misko:swe;shyam:set;');
   }));
 
-
-  it('should ngRepeat over object with primitive value correctly', inject(function($rootScope, $compile) {
+
+  it('should ngRepeat over object with changing primitive value',
+      inject(function($rootScope, $compile) {
+
     element = $compile(
       '<ul>' +
-        '<li ng-repeat="(key, value) in items" ng-bind="key + \':\' + value + \';\' "></li>' +
+        '<li ng-repeat="(key, value) in items">' +
+          '{{key}}:{{value}};' +
+          '<input type="checkbox" ng-model="items[key]">' +
+        '</li>' +
       '</ul>')($rootScope);
-    $rootScope.items = {misko:'true', shyam:'true', zhenbo: 'true'};
+    //document.body.appendChild(element[0]);
+    $rootScope.items = {misko: true, shyam: true, zhenbo:true};
     $rootScope.$digest();
     expect(element.find('li').length).toEqual(3);
     expect(element.text()).toEqual('misko:true;shyam:true;zhenbo:true;');
-
-    $rootScope.items = {misko:'false', shyam:'true', zhenbo: 'true'};
-    $rootScope.$digest();
-    expect(element.find('li').length).toEqual(3);
+
+    browserTrigger(element.find('input').eq(0), 'click');
+
     expect(element.text()).toEqual('misko:false;shyam:true;zhenbo:true;');
-
-    $rootScope.items = {misko:'false', shyam:'false', zhenbo: 'false'};
-    $rootScope.$digest();
-    expect(element.find('li').length).toEqual(3);
-    expect(element.text()).toEqual('misko:false;shyam:false;zhenbo:false;');
-
-    $rootScope.items = {misko:'true'};
-    $rootScope.$digest();
-    expect(element.find('li').length).toEqual(1);
-    expect(element.text()).toEqual('misko:true;');
+    expect(element.find('input')[0].checked).toBe(false);
+    expect(element.find('input')[1].checked).toBe(true);
+    expect(element.find('input')[2].checked).toBe(true);
 
-    $rootScope.items = {shyam:'true', zhenbo: 'false'};
+    browserTrigger(element.find('input').eq(0), 'click');
+    expect(element.text()).toEqual('misko:true;shyam:true;zhenbo:true;');
+    expect(element.find('input')[0].checked).toBe(true);
+    expect(element.find('input')[1].checked).toBe(true);
+    expect(element.find('input')[2].checked).toBe(true);
+
+    browserTrigger(element.find('input').eq(1), 'click');
+    expect(element.text()).toEqual('misko:true;shyam:false;zhenbo:true;');
+    expect(element.find('input')[0].checked).toBe(true);
+    expect(element.find('input')[1].checked).toBe(false);
+    expect(element.find('input')[2].checked).toBe(true);
+
+    $rootScope.items = {misko: false, shyam: true, zhenbo: true};
     $rootScope.$digest();
-    expect(element.find('li').length).toEqual(2);
-    expect(element.text()).toEqual('shyam:true;zhenbo:false;');
+    expect(element.text()).toEqual('misko:false;shyam:true;zhenbo:true;');
+    expect(element.find('input')[0].checked).toBe(false);
+    expect(element.find('input')[1].checked).toBe(true);
+    expect(element.find('input')[2].checked).toBe(true);
   }));