Skip to content

Commit f417df8

Browse files
Making watchCollection call the listener before clobbering oldValue
I took the naive approach suggested in angular#2621 of separating out the changeDetected logic from the rest of the watch to give the listener time to execute before oldCollection is overwritten. The logic should be identical to what's in the main repo, except no changes are made to the closed-over state until after listener is called back. Unfortunately, this doesn't appear to work, as the watchCollection tests are now all failing. I need to learn more about Karma so I can better debug this, and take more time to grok the logic of the function before I'd have a shot at fixing it.
1 parent 9814c75 commit f417df8

File tree

1 file changed

+40
-14
lines changed

1 file changed

+40
-14
lines changed

src/ng/rootScope.js

+40-14
Original file line numberDiff line numberDiff line change
@@ -365,32 +365,67 @@ function $RootScopeProvider(){
365365

366366
function $watchCollectionWatch() {
367367
newValue = objGetter(self);
368+
369+
if (!isObject(newValue) && oldValue !== newValue) {
370+
return changeDetected++;
371+
}
372+
373+
if (isArrayLike(newValue)) {
374+
var newLength = newValue.length;
375+
376+
if (oldValue !== internalArray || oldValue.length != newLength) {
377+
return changeDetected++;
378+
}
379+
380+
for (var i = 0; i < newLength; i++) {
381+
if (oldValue[i] !== newValue[i]) {
382+
return changeDetected++;
383+
}
384+
}
385+
386+
} else {
387+
if (oldValue !== internalObject) {
388+
return changeDetected++;
389+
}
390+
391+
for (var key in newValue) {
392+
if (
393+
newValue.hasOwnProperty(key) &&
394+
(!oldValue.hasOwnProperty(key) || oldValue[key] !== newValue[key])
395+
) {
396+
return changeDetected++;
397+
}
398+
}
399+
}
400+
}
401+
402+
function $watchCollectionAction() {
368403
var newLength, key;
369404

405+
newValue = objGetter(self);
406+
407+
listener(newValue, oldValue, self);
408+
370409
if (!isObject(newValue)) {
371410
if (oldValue !== newValue) {
372411
oldValue = newValue;
373-
changeDetected++;
374412
}
375413
} else if (isArrayLike(newValue)) {
376414
if (oldValue !== internalArray) {
377415
// we are transitioning from something which was not an array into array.
378416
oldValue = internalArray;
379417
oldLength = oldValue.length = 0;
380-
changeDetected++;
381418
}
382419

383420
newLength = newValue.length;
384421

385422
if (oldLength !== newLength) {
386423
// if lengths do not match we need to trigger change notification
387-
changeDetected++;
388424
oldValue.length = oldLength = newLength;
389425
}
390426
// copy the items to oldValue and look for changes.
391427
for (var i = 0; i < newLength; i++) {
392428
if (oldValue[i] !== newValue[i]) {
393-
changeDetected++;
394429
oldValue[i] = newValue[i];
395430
}
396431
}
@@ -399,7 +434,6 @@ function $RootScopeProvider(){
399434
// we are transitioning from something which was not an object into object.
400435
oldValue = internalObject = {};
401436
oldLength = 0;
402-
changeDetected++;
403437
}
404438
// copy the items to oldValue and look for changes.
405439
newLength = 0;
@@ -408,32 +442,24 @@ function $RootScopeProvider(){
408442
newLength++;
409443
if (oldValue.hasOwnProperty(key)) {
410444
if (oldValue[key] !== newValue[key]) {
411-
changeDetected++;
412445
oldValue[key] = newValue[key];
413446
}
414447
} else {
415448
oldLength++;
416449
oldValue[key] = newValue[key];
417-
changeDetected++;
418450
}
419451
}
420452
}
421453
if (oldLength > newLength) {
422454
// we used to have more keys, need to find them and destroy them.
423-
changeDetected++;
424-
for(key in oldValue) {
455+
for (key in oldValue) {
425456
if (oldValue.hasOwnProperty(key) && !newValue.hasOwnProperty(key)) {
426457
oldLength--;
427458
delete oldValue[key];
428459
}
429460
}
430461
}
431462
}
432-
return changeDetected;
433-
}
434-
435-
function $watchCollectionAction() {
436-
listener(newValue, oldValue, self);
437463
}
438464

439465
return this.$watch($watchCollectionWatch, $watchCollectionAction);

0 commit comments

Comments
 (0)