angular · Oct 26, 2011 · Oct 26, 2011
Showing with 78 additions and 46 deletions.

+1 −1 src/apis.js

+55 −43 src/widgets.js

+22 −2 test/widgetsSpec.js
diff --git a/src/apis.js b/src/apis.js
@@ -936,7 +936,7 @@ HashMap.prototype = {
 };
 
 /**
- * A map where multiple values can be added to the same key such that the form a queue.
+ * A map where multiple values can be added to the same key such that they form a queue.
  * @returns {HashQueueMap}
  */
 function HashQueueMap() {}

diff --git a/src/widgets.js b/src/widgets.js
@@ -361,7 +361,7 @@ angularWidget('@ng:repeat', function(expression, element){
     // We expect this to be a rare case.
     var lastOrder = new HashQueueMap();
     this.$watch(function(scope){
-      var index = 0,
+      var index, length,
           collection = scope.$eval(rhs),
           collectionLength = size(collection, true),
           childScope,
@@ -372,52 +372,64 @@ angularWidget('@ng:repeat', function(expression, element){
           array, last,       // last object information {scope, element, index}
           cursor = iterStartElement;     // current position of the node
 
-      for (key in collection) {
-        if (collection.hasOwnProperty(key) && key.charAt(0) != '$') {
-          last = lastOrder.shift(value = collection[key]);
-          if (last) {
-            // if we have already seen this object, then we need to reuse the
-            // associated scope/element
-            childScope = last.scope;
-            nextOrder.push(value, last);
-
-            if (index === last.index) {
-              // do nothing
-              cursor = last.element;
-            } else {
-              // existing item which got moved
-              last.index = index;
-              // This may be a noop, if the element is next, but I don't know of a good way to
-              // figure this out,  since it would require extra DOM access, so let's just hope that
-              // the browsers realizes that it is noop, and treats it as such.
-              cursor.after(last.element);
-              cursor = last.element;
-            }
-          } else {
-            // new item which we don't know about
-            childScope = parentScope.$new();
+      if (!isArray(collection)) {
+        // if object, extract keys, sort them and use to determine order of iteration over obj props
+        array = [];
+        for(key in collection) {
+          if (collection.hasOwnProperty(key) && key.charAt(0) != '$') {
+            array.push(key);
           }
+        }
+        array.sort();
+      } else {
+        array = collection || [];
+      }
 
-          childScope[valueIdent] = collection[key];
-          if (keyIdent) childScope[keyIdent] = key;
-          childScope.$index = index;
-          childScope.$position = index == 0
-              ? 'first'
-              : (index == collectionLength - 1 ? 'last' : 'middle');
-
-          if (!last) {
-            linker(childScope, function(clone){
-              cursor.after(clone);
-              last = {
-                  scope: childScope,
-                  element: (cursor = clone),
-                  index: index
-                };
-              nextOrder.push(value, last);
-            });
+      // we are not using forEach for perf reasons (trying to avoid #call)
+      for (index = 0, length = array.length; index < length; index++) {
+        key = (collection === array) ? index : array[index];
+        value = collection[key];
+        last = lastOrder.shift(value);
+        if (last) {
+          // if we have already seen this object, then we need to reuse the
+          // associated scope/element
+          childScope = last.scope;
+          nextOrder.push(value, last);
+
+          if (index === last.index) {
+            // do nothing
+            cursor = last.element;
+          } else {
+            // existing item which got moved
+            last.index = index;
+            // This may be a noop, if the element is next, but I don't know of a good way to
+            // figure this out,  since it would require extra DOM access, so let's just hope that
+            // the browsers realizes that it is noop, and treats it as such.
+            cursor.after(last.element);
+            cursor = last.element;
           }
+        } else {
+          // new item which we don't know about
+          childScope = parentScope.$new();
+        }
 
-          index ++;
+        childScope[valueIdent] = value;
+        if (keyIdent) childScope[keyIdent] = key;
+        childScope.$index = index;
+        childScope.$position = index == 0
+            ? 'first'
+            : (index == collectionLength - 1 ? 'last' : 'middle');
+
+        if (!last) {
+          linker(childScope, function(clone){
+            cursor.after(clone);
+            last = {
+                scope: childScope,
+                element: (cursor = clone),
+                index: index
+              };
+            nextOrder.push(value, last);
+          });
         }
       }
 

diff --git a/test/widgetsSpec.js b/test/widgetsSpec.js
@@ -254,7 +254,7 @@ describe("widget", function() {
                                   'ng:bind="key + \':\' + val + $index + \'|\'"></li></ul>');
       scope.items = {'misko':'m', 'shyam':'s', 'frodo':'f'};
       scope.$digest();
-      expect(element.text()).toEqual('misko:m0|shyam:s1|frodo:f2|');
+      expect(element.text()).toEqual('frodo:f0|misko:m1|shyam:s2|');
     });
 
     it('should expose iterator position as $position when iterating over arrays', function() {
@@ -282,7 +282,7 @@ describe("widget", function() {
         '</ul>');
       scope.items = {'misko':'m', 'shyam':'s', 'doug':'d', 'frodo':'f'};
       scope.$digest();
-      expect(element.text()).toEqual('misko:m:first|shyam:s:middle|doug:d:middle|frodo:f:last|');
+      expect(element.text()).toEqual('doug:d:first|frodo:f:middle|misko:m:middle|shyam:s:last|');
 
       delete scope.items.doug;
       delete scope.items.frodo;
@@ -312,6 +312,26 @@ describe("widget", function() {
       expect(element.text()).toEqual('a|b|Xc|d|X');
     });
 
+    it('should ignore non-array element properties when iterating over an array', function() {
+      var scope = compile('<ul><li ng:repeat="item in array">{{item}}|</li></ul>');
+      scope.array = ['a', 'b', 'c'];
+      scope.array.foo = '23';
+      scope.array.bar = function() {};
+      scope.$digest();
+
+      expect(element.text()).toBe('a|b|c|');
+    });
+
+    it('should iterate over non-existent elements of a sparse array', function() {
+      var scope = compile('<ul><li ng:repeat="item in array">{{item}}|</li></ul>');
+      scope.array = ['a', 'b'];
+      scope.array[4] = 'c';
+      scope.array[6] = 'd';
+      scope.$digest();
+
+      expect(element.text()).toBe('a|b|||c||d|');
+    });
+
 
     describe('stability', function() {
       var a, b, c, d, scope, lis;