fix(collectionRepeat): simplify item reusing process to fix rare reuse error

Closes #1777.

js/angular/service/collectionRepeatDataSource.js

@@ -25,26 +25,14 @@ function($cacheFactory, $parse, $rootScope) {
     this.dimensions = [];
     this.data = [];
 
-    if (this.trackByExpr) {
-      var trackByGetter = $parse(this.trackByExpr);
-      var hashFnLocals = {$id: hashKey};
-      this.itemHashGetter = function(index, value) {
-        hashFnLocals[self.keyExpr] = value;
-        hashFnLocals.$index = index;
-        return trackByGetter(self.scope, hashFnLocals);
-      };
-    } else {
-      this.itemHashGetter = function(index, value) {
-        return hashKey(value);
-      };
-    }
-
     this.attachedItems = {};
-    this.BACKUP_ITEMS_LENGTH = 10;
+    this.BACKUP_ITEMS_LENGTH = 20;
     this.backupItemsArray = [];
   }
   CollectionRepeatDataSource.prototype = {
     setup: function() {
+      if (this.isSetup) return;
+      this.isSetup = true;
       for (var i = 0; i < this.BACKUP_ITEMS_LENGTH; i++) {
         this.detachItem(this.createItem());
       }
@@ -70,19 +58,18 @@ function($cacheFactory, $parse, $rootScope) {
     },
     createItem: function() {
       var item = {};
-      item.scope = this.scope.$new();
 
+      item.scope = this.scope.$new();
       this.transcludeFn(item.scope, function(clone) {
         clone.css('position', 'absolute');
         item.element = clone;
       });
-
       this.transcludeParent.append(item.element);
 
       return item;
     },
-    getItem: function(hash) {
-      if ( (item = this.attachedItems[hash]) ) {
+    getItem: function(index) {
+      if ( (item = this.attachedItems[index]) ) {
         //do nothing, the item is good
       } else if ( (item = this.backupItemsArray.pop()) ) {
         reconnectScope(item.scope);
@@ -92,20 +79,18 @@ function($cacheFactory, $parse, $rootScope) {
       return item;
     },
     attachItemAtIndex: function(index) {
-      var value = this.data[index];
-
       if (index < this.dataStartIndex) {
         return this.beforeSiblings[index];
       } else if (index > this.data.length) {
         return this.afterSiblings[index - this.data.length - this.dataStartIndex];
       }
 
-      var hash = this.itemHashGetter(index, value);
-      var item = this.getItem(hash);
+      var item = this.getItem(index);
+      var value = this.data[index];
 
-      if (item.scope.$index !== index || item.scope[this.keyExpr] !== value) {
+      if (item.index !== index || item.scope[this.keyExpr] !== value) {
+        item.index = item.scope.$index = index;
         item.scope[this.keyExpr] = value;
-        item.scope.$index = index;
         item.scope.$first = (index === 0);
         item.scope.$last = (index === (this.getLength() - 1));
         item.scope.$middle = !(item.scope.$first || item.scope.$last);
@@ -116,9 +101,7 @@ function($cacheFactory, $parse, $rootScope) {
           item.scope.$digest();
         }
       }
-
-      item.hash = hash;
-      this.attachedItems[hash] = item;
+      this.attachedItems[index] = item;
 
       return item;
     },
@@ -129,13 +112,12 @@ function($cacheFactory, $parse, $rootScope) {
       item.element = null;
     },
     detachItem: function(item) {
-      delete this.attachedItems[item.hash];
+      delete this.attachedItems[item.index];
 
       //If it's an outside item, only hide it. These items aren't part of collection
       //repeat's list, only sit outside
       if (item.isOutside) {
         hideWithTransform(item.element);
-
       // If we are at the limit of backup items, just get rid of the this element
       } else if (this.backupItemsArray.length >= this.BACKUP_ITEMS_LENGTH) {
         this.destroyItem(item);
@@ -167,36 +149,6 @@ function($cacheFactory, $parse, $rootScope) {
   return CollectionRepeatDataSource;
 }]);
 
-/**
-* Computes a hash of an 'obj'.
- * Hash of a:
- *  string is string
- *  number is number as string
- *  object is either result of calling $$hashKey function on the object or uniquely generated id,
- *         that is also assigned to the $$hashKey property of the object.
- *
- * @param obj
- * @returns {string} hash string such that the same input will have the same hash string.
- *         The resulting string key is in 'type:hashKey' format.
- */
-function hashKey(obj) {
-  var objType = typeof obj,
-      key;
-
-  if (objType == 'object' && obj !== null) {
-    if (typeof (key = obj.$$hashKey) == 'function') {
-      // must invoke on object to keep the right this
-      key = obj.$$hashKey();
-    } else if (key === undefined) {
-      key = obj.$$hashKey = ionic.Utils.nextUid();
-    }
-  } else {
-    key = obj;
-  }
-
-  return objType + ':' + key;
-}
-
 function disconnectScope(scope) {
   if (scope.$root === scope) {
     return; // we can't disconnect the root node;

js/angular/service/collectionRepeatManager.js

@@ -155,9 +155,7 @@ function($rootScope, $timeout) {
       this.beforeSize = result.beforeSize;
       this.setCurrentIndex(0);
       this.render(true);
-      if (!this.dataSource.backupItemsArray.length) {
-        this.dataSource.setup();
-      }
+      this.dataSource.setup();
     },
     /*
      * setCurrentIndex sets the index in the list that matches the scroller's position.
@@ -194,6 +192,7 @@ function($rootScope, $timeout) {
       }
       return this.scrollView.__$callback(transformLeft, transformTop, zoom, wasResize);
     }),
+
     renderIfNeeded: function(scrollPos) {
       if ((this.hasNextIndex && scrollPos >= this.nextPos) ||
           (this.hasPrevIndex && scrollPos < this.previousPos)) {
@@ -334,15 +333,6 @@ function($rootScope, $timeout) {
     }
   };
 
-  var exceptions = {'renderScroll':1, 'renderIfNeeded':1};
-  forEach(CollectionRepeatManager.prototype, function(method, key) {
-    if (exceptions[key]) return;
-    CollectionRepeatManager.prototype[key] = function() {
-      console.log(key + '(', arguments, ')');
-      return method.apply(this, arguments);
-    };
-  });
-
   return CollectionRepeatManager;
 }]);
 

test/html/list-fit.html

@@ -59,11 +59,11 @@ function MainCtrl($scope, $ionicScrollDelegate, $timeout, $q, $ionicLoading) {
 
   $scope.loadMore = function() {
     $timeout(function() {
-      var n = 1 + Math.floor(4*Math.random());
+      var n = 100;//1 + Math.floor(4*Math.random());
       for (var i = 0; i < n; i++) addImage();
       $scope.$broadcast('scroll.infiniteScrollComplete');
       $scope.$broadcast('scroll.refreshComplete');
-    }, 1500);
+    }, 1);
   };
 }
   </script>

test/unit/angular/service/collectionDataSource.unit.js

@@ -35,7 +35,6 @@ describe('$collectionDataSource service', function() {
       transcludeParent: 3,
       keyExpr: 4,
       listExpr: 5,
-      trackByExpr: 6,
       heightGetter: 7,
       widthGetter: 8
     });
@@ -44,35 +43,10 @@ describe('$collectionDataSource service', function() {
     expect(source.transcludeParent).toBe(3);
     expect(source.keyExpr).toBe(4);
     expect(source.listExpr).toBe(5);
-    expect(source.trackByExpr).toBe(6);
     expect(source.heightGetter).toBe(7);
     expect(source.widthGetter).toBe(8);
   }));
 
-  describe('.itemHashGetter()', function() {
-
-    it('should default to hashKey of value', function() {
-      spyOn(window, 'hashKey').andReturn('hashed');
-
-      var source = setup();
-      expect(source.itemHashGetter(1, 2)).toBe('hashed');
-      expect(hashKey).toHaveBeenCalledWith(2);
-    });
-
-    it('should use trackbyExpr if provided, and provide locals', function() {
-      var source = setup({
-        keyExpr: 'item',
-        trackByExpr: 'item.idMaker($index, item)'
-      });
-      var idMakerSpy = jasmine.createSpy('idMaker').andReturn('superhash');
-      var item = {
-        idMaker: idMakerSpy
-      };
-      expect(source.itemHashGetter(1, item)).toEqual('superhash');
-      expect(idMakerSpy).toHaveBeenCalledWith(1, item);
-    });
-  });
-
   it('.calculateDataDimensions()', function() {
     function widthGetter(scope, locals) {
       return locals.$index + locals.item + 'w';
@@ -119,7 +93,7 @@ describe('$collectionDataSource service', function() {
   });
 
   describe('.getItem()', function() {
-    it('should return attachedItems[hash] if available', function() {
+    it('should return attachedItems[index] if available', function() {
       var source = setup();
       var item = {};
       source.attachedItems['123'] = item;
@@ -155,9 +129,6 @@ describe('$collectionDataSource service', function() {
       spyOn(source, 'getItem').andCallFake(function() {
         return { scope: $rootScope.$new() };
       });
-      spyOn(source, 'itemHashGetter').andCallFake(function(index, value) {
-        return index + ':' + value;
-      });
 
       var item1 = source.attachItemAtIndex(0);
       expect(item1.scope.value).toEqual('a');
@@ -166,7 +137,6 @@ describe('$collectionDataSource service', function() {
       expect(item1.scope.$last).toBe(false);
       expect(item1.scope.$middle).toBe(false);
       expect(item1.scope.$odd).toBe(false);
-      expect(item1.hash).toEqual('0:a');
 
       var item2 = source.attachItemAtIndex(1);
       expect(item2.scope.value).toEqual('b');
@@ -175,7 +145,6 @@ describe('$collectionDataSource service', function() {
       expect(item2.scope.$last).toBe(false);
       expect(item2.scope.$middle).toBe(true);
       expect(item2.scope.$odd).toBe(true);
-      expect(item2.hash).toEqual('1:b');
 
       var item3 = source.attachItemAtIndex(2);
       expect(item3.scope.value).toEqual('c');
@@ -184,12 +153,11 @@ describe('$collectionDataSource service', function() {
       expect(item3.scope.$last).toBe(true);
       expect(item3.scope.$middle).toBe(false);
       expect(item3.scope.$odd).toBe(false);
-      expect(item3.hash).toEqual('2:c');
 
       expect(source.attachedItems).toEqual({
-        '0:a': item1,
-        '1:b': item2,
-        '2:c': item3
+        0: item1,
+        1: item2,
+        2: item3
       });
     }));
   });
@@ -200,10 +168,10 @@ describe('$collectionDataSource service', function() {
       var item = {
         element: angular.element('<div>'),
         scope: {},
-        hash: 'foo'
+        index: 1
       };
       source.backupItemsArray = [];
-      source.attachedItems[item.hash] = item;
+      source.attachedItems[1] = item;
       spyOn(window, 'disconnectScope');
       source.detachItem(item);
       expect(source.attachedItems).toEqual({});
@@ -215,8 +183,8 @@ describe('$collectionDataSource service', function() {
       spyOn(source, 'destroyItem');
       source.BACKUP_ITEMS_LENGTH = 0;
 
-      var item = { hash: 'abc' };
-      source.attachedItems[item.hash] = item;
+      var item = { index: 0 };
+      source.attachedItems[item.index] = item;
       source.detachItem(item);
       expect(source.destroyItem).toHaveBeenCalledWith(item);
       expect(source.attachedItems).toEqual({});