fix(Scope): $watchCollection should call listener with oldValue

Originally we destroyed the oldValue by incrementaly copying over portions of the newValue
into the oldValue during dirty-checking, this resulted in oldValue to be equal to newValue
by the time we called the watchCollection listener.

The fix creates a copy of the newValue each time a change is detected and then uses that
copy *the next time* a change is detected.

To make `$watchCollection` behave the same way as `$watch`, during the first iteration
the listener is called with newValue and oldValue being identical.

Since many of the corner-cases are already covered by existing tests, I refactored the
test logging to include oldValue and made the tests more readable.

Closes #2621
Closes #5661
Closes #5688
Closes #6736
This commit is contained in:
Igor Minar
2014-03-17 15:48:09 -07:00
parent 922cb7e42f
commit 3dd9572754
2 changed files with 119 additions and 46 deletions

View File

@@ -483,104 +483,127 @@ describe('Scope', function() {
describe('$watchCollection', function() {
var log, $rootScope, deregister;
beforeEach(inject(function(_$rootScope_) {
log = [];
beforeEach(inject(function(_$rootScope_, _log_) {
$rootScope = _$rootScope_;
deregister = $rootScope.$watchCollection('obj', function logger(obj) {
log.push(toJson(obj));
log = _log_;
deregister = $rootScope.$watchCollection('obj', function logger(newVal, oldVal) {
var msg = {newVal: newVal, oldVal: oldVal};
if (newVal === oldVal) {
msg.identical = true;
}
log(msg);
});
}));
it('should not trigger if nothing change', inject(function($rootScope) {
$rootScope.$digest();
expect(log).toEqual([undefined]);
expect(log).toEqual([{ newVal : undefined, oldVal : undefined, identical : true }]);
log.reset();
$rootScope.$digest();
expect(log).toEqual([undefined]);
expect(log).toEqual([]);
}));
it('should allow deregistration', inject(function($rootScope) {
it('should allow deregistration', function() {
$rootScope.obj = [];
$rootScope.$digest();
expect(log).toEqual(['[]']);
expect(log.toArray().length).toBe(1);
log.reset();
$rootScope.obj.push('a');
deregister();
$rootScope.$digest();
expect(log).toEqual(['[]']);
}));
expect(log).toEqual([]);
});
describe('array', function() {
it('should return oldCollection === newCollection only on the first listener call',
inject(function($rootScope, log) {
// first time should be identical
$rootScope.obj = ['a', 'b'];
$rootScope.$digest();
expect(log).toEqual([{newVal: ['a', 'b'], oldVal: ['a', 'b'], identical: true}]);
log.reset();
// second time should be different
$rootScope.obj[1] = 'c';
$rootScope.$digest();
expect(log).toEqual([{newVal: ['a', 'c'], oldVal: ['a', 'b']}]);
}));
it('should trigger when property changes into array', function() {
$rootScope.obj = 'test';
$rootScope.$digest();
expect(log).toEqual(['"test"']);
expect(log.empty()).toEqual([{newVal: "test", oldVal: "test", identical: true}]);
$rootScope.obj = [];
$rootScope.$digest();
expect(log).toEqual(['"test"', '[]']);
expect(log.empty()).toEqual([{newVal: [], oldVal: "test"}]);
$rootScope.obj = {};
$rootScope.$digest();
expect(log).toEqual(['"test"', '[]', '{}']);
expect(log.empty()).toEqual([{newVal: {}, oldVal: []}]);
$rootScope.obj = [];
$rootScope.$digest();
expect(log).toEqual(['"test"', '[]', '{}', '[]']);
expect(log.empty()).toEqual([{newVal: [], oldVal: {}}]);
$rootScope.obj = undefined;
$rootScope.$digest();
expect(log).toEqual(['"test"', '[]', '{}', '[]', undefined]);
expect(log.empty()).toEqual([{newVal: undefined, oldVal: []}]);
});
it('should not trigger change when object in collection changes', function() {
$rootScope.obj = [{}];
$rootScope.$digest();
expect(log).toEqual(['[{}]']);
expect(log.empty()).toEqual([{newVal: [{}], oldVal: [{}], identical: true}]);
$rootScope.obj[0].name = 'foo';
$rootScope.$digest();
expect(log).toEqual(['[{}]']);
expect(log).toEqual([]);
});
it('should watch array properties', function() {
$rootScope.obj = [];
$rootScope.$digest();
expect(log).toEqual(['[]']);
expect(log.empty()).toEqual([{newVal: [], oldVal: [], identical: true}]);
$rootScope.obj.push('a');
$rootScope.$digest();
expect(log).toEqual(['[]', '["a"]']);
expect(log.empty()).toEqual([{newVal: ['a'], oldVal: []}]);
$rootScope.obj[0] = 'b';
$rootScope.$digest();
expect(log).toEqual(['[]', '["a"]', '["b"]']);
expect(log.empty()).toEqual([{newVal: ['b'], oldVal: ['a']}]);
$rootScope.obj.push([]);
$rootScope.obj.push({});
log = [];
$rootScope.$digest();
expect(log).toEqual(['["b",[],{}]']);
expect(log.empty()).toEqual([{newVal: ['b', [], {}], oldVal: ['b']}]);
var temp = $rootScope.obj[1];
$rootScope.obj[1] = $rootScope.obj[2];
$rootScope.obj[2] = temp;
$rootScope.$digest();
expect(log).toEqual([ '["b",[],{}]', '["b",{},[]]' ]);
expect(log.empty()).toEqual([{newVal: ['b', {}, []], oldVal: ['b', [], {}]}]);
$rootScope.obj.shift();
log = [];
$rootScope.$digest();
expect(log).toEqual([ '[{},[]]' ]);
expect(log.empty()).toEqual([{newVal: [{}, []], oldVal: ['b', {}, []]}]);
});
it('should watch array-like objects like arrays', function () {
var arrayLikelog = [];
$rootScope.$watchCollection('arrayLikeObject', function logger(obj) {
@@ -601,57 +624,72 @@ describe('Scope', function() {
describe('object', function() {
it('should return oldCollection === newCollection only on the first listener call',
inject(function($rootScope, log) {
$rootScope.obj = {'a': 'b'};
// first time should be identical
$rootScope.$digest();
expect(log.empty()).toEqual([{newVal: {'a': 'b'}, oldVal: {'a': 'b'}, identical: true}]);
// second time not identical
$rootScope.obj.a = 'c';
$rootScope.$digest();
expect(log).toEqual([{newVal: {'a': 'c'}, oldVal: {'a': 'b'}}]);
}));
it('should trigger when property changes into object', function() {
$rootScope.obj = 'test';
$rootScope.$digest();
expect(log).toEqual(['"test"']);
expect(log.empty()).toEqual([{newVal: 'test', oldVal: 'test', identical: true}]);
$rootScope.obj = {};
$rootScope.$digest();
expect(log).toEqual(['"test"', '{}']);
expect(log.empty()).toEqual([{newVal: {}, oldVal: 'test'}]);
});
it('should not trigger change when object in collection changes', function() {
$rootScope.obj = {name: {}};
$rootScope.$digest();
expect(log).toEqual(['{"name":{}}']);
expect(log.empty()).toEqual([{newVal: {name: {}}, oldVal: {name: {}}, identical: true}]);
$rootScope.obj.name.bar = 'foo';
$rootScope.$digest();
expect(log).toEqual(['{"name":{}}']);
expect(log.empty()).toEqual([]);
});
it('should watch object properties', function() {
$rootScope.obj = {};
$rootScope.$digest();
expect(log).toEqual(['{}']);
expect(log.empty()).toEqual([{newVal: {}, oldVal: {}, identical: true}]);
$rootScope.obj.a= 'A';
$rootScope.$digest();
expect(log).toEqual(['{}', '{"a":"A"}']);
expect(log.empty()).toEqual([{newVal: {a: 'A'}, oldVal: {}}]);
$rootScope.obj.a = 'B';
$rootScope.$digest();
expect(log).toEqual(['{}', '{"a":"A"}', '{"a":"B"}']);
expect(log.empty()).toEqual([{newVal: {a: 'B'}, oldVal: {a: 'A'}}]);
$rootScope.obj.b = [];
$rootScope.obj.c = {};
log = [];
$rootScope.$digest();
expect(log).toEqual(['{"a":"B","b":[],"c":{}}']);
expect(log.empty()).toEqual([{newVal: {a: 'B', b: [], c: {}}, oldVal: {a: 'B'}}]);
var temp = $rootScope.obj.a;
$rootScope.obj.a = $rootScope.obj.b;
$rootScope.obj.c = temp;
$rootScope.$digest();
expect(log).toEqual([ '{"a":"B","b":[],"c":{}}', '{"a":[],"b":[],"c":"B"}' ]);
expect(log.empty()).
toEqual([{newVal: {a: [], b: {}, c: 'B'}, oldVal: {a: 'B', b: [], c: {}}}]);
delete $rootScope.obj.a;
log = [];
$rootScope.$digest();
expect(log).toEqual([ '{"b":[],"c":"B"}' ]);
expect(log.empty()).toEqual([{newVal: {b: {}, c: 'B'}, oldVal: {a: [], b: {}, c: 'B'}}]);
});
});
});