fix(ng:repeat) with array ignore properties not representing array elements

Along the way I also changed the repeater impl to use for loop instead
of for in loop.

Iteration over objects is handled by creating an array of keys, which is
sorted and this array then determines the order of iteration over an
element. This makes repeating over objects deterministic and
cross-browser compatible.
This commit is contained in:
Igor Minar 2011-10-24 09:18:44 -07:00
parent d5ccabce60
commit 3945f884c5
2 changed files with 76 additions and 44 deletions

View file

@ -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 (!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 || [];
}
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;
}
// 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 {
// new item which we don't know about
childScope = parentScope.$new();
// 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();
}
childScope[valueIdent] = collection[key];
if (keyIdent) childScope[keyIdent] = key;
childScope.$index = index;
childScope.$position = index == 0
? 'first'
: (index == collectionLength - 1 ? 'last' : 'middle');
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);
});
}
index ++;
if (!last) {
linker(childScope, function(clone){
cursor.after(clone);
last = {
scope: childScope,
element: (cursor = clone),
index: index
};
nextOrder.push(value, last);
});
}
}

View file

@ -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;