Skip to content

Commit 8df788f

Browse files
committed
Merge pull request emberjs#3361 from hjdivad/fix-3331
Fire observers for array computed changes.
2 parents 8624e85 + 56087af commit 8df788f

File tree

2 files changed

+77
-3
lines changed

2 files changed

+77
-3
lines changed

packages/ember-runtime/lib/computed/reduce_computed.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ var e_get = Ember.get,
55
set = Ember.set,
66
guidFor = Ember.guidFor,
77
metaFor = Ember.meta,
8+
propertyWillChange = Ember.propertyWillChange,
9+
propertyDidChange = Ember.propertyDidChange,
810
addBeforeObserver = Ember.addBeforeObserver,
911
removeBeforeObserver = Ember.removeBeforeObserver,
1012
addObserver = Ember.addObserver,
@@ -75,7 +77,7 @@ function ItemPropertyObserverContext (dependentArray, index, trackedArray) {
7577

7678
DependentArraysObserver.prototype = {
7779
setValue: function (newValue) {
78-
this.instanceMeta.setValue(newValue);
80+
this.instanceMeta.setValue(newValue, true);
7981
},
8082
getValue: function () {
8183
return this.instanceMeta.getValue();
@@ -370,11 +372,21 @@ ReduceComputedPropertyInstanceMeta.prototype = {
370372
}
371373
},
372374

373-
setValue: function(newValue) {
375+
setValue: function(newValue, triggerObservers) {
374376
// This lets sugars force a recomputation, handy for very simple
375377
// implementations of eg max.
376378
if (newValue !== undefined) {
379+
var fireObservers = triggerObservers && (newValue !== this.cache[this.propertyName]);
380+
381+
if (fireObservers) {
382+
propertyWillChange(this.context, this.propertyName);
383+
}
384+
377385
this.cache[this.propertyName] = newValue;
386+
387+
if (fireObservers) {
388+
propertyDidChange(this.context, this.propertyName);
389+
}
378390
} else {
379391
delete this.cache[this.propertyName];
380392
}
@@ -640,6 +652,12 @@ ReduceComputedProperty.prototype.property = function () {
640652
to invalidate the computation. This is generally not a good idea for
641653
arrayComputed but it's used in eg max and min.
642654
655+
Note that observers will be fired if either of these functions return a value
656+
that differs from the accumulated value. When returning an object that
657+
mutates in response to array changes, for example an array that maps
658+
everything from some other array (see `Ember.computed.map`), it is usually
659+
important that the *same* array be returned to avoid accidentally triggering observers.
660+
643661
Example
644662
645663
```javascript

packages/ember-runtime/tests/computed/reduce_computed_macros_test.js

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,23 @@ test("it maps properties", function() {
168168
deepEqual(get(obj, 'mapped'), [1, 3, 2, 5]);
169169
});
170170

171+
test("it is observerable", function() {
172+
var mapped = get(obj, 'mapped'),
173+
calls = 0;
174+
175+
deepEqual(get(obj, 'mapped'), [1, 3, 2, 1]);
176+
177+
Ember.addObserver(obj, 'mapped.@each', function() {
178+
calls++;
179+
});
180+
181+
Ember.run(function() {
182+
obj.get('array').pushObject({ v: 5 });
183+
});
184+
185+
equal(calls, 1, 'Ember.computed.mapBy is observerable');
186+
});
187+
171188

172189
module('Ember.computed.filter', {
173190
setup: function() {
@@ -1100,7 +1117,6 @@ test("filtering, sorting and reduce (max) can be combined", function() {
11001117
equal(35, get(obj, 'oldestStarkAge'), "chain is updated correctly");
11011118
});
11021119

1103-
11041120
function todo(name, priority) {
11051121
return Ember.Object.create({name: name, priority: priority});
11061122
}
@@ -1160,3 +1176,43 @@ test("it can filter and sort when both depend on the same item property", functi
11601176
deepEqual(sorted.mapProperty('name'), ['A', 'B', 'C', 'E', 'D'], "precond - sorted updated correctly");
11611177
deepEqual(filtered.mapProperty('name'), ['A', 'C', 'E', 'D'], "filtered updated correctly");
11621178
});
1179+
1180+
module('Chaining array and reduced CPs', {
1181+
setup: function() {
1182+
Ember.run(function() {
1183+
userFnCalls = 0;
1184+
obj = Ember.Object.createWithMixins({
1185+
array: Ember.A([{ v: 1 }, { v: 3}, { v: 2 }, { v: 1 }]),
1186+
mapped: Ember.computed.mapBy('array', 'v'),
1187+
max: Ember.computed.max('mapped'),
1188+
maxDidChange: Ember.observer(function(){
1189+
userFnCalls++;
1190+
},'max')
1191+
});
1192+
});
1193+
},
1194+
teardown: function() {
1195+
Ember.run(function() {
1196+
obj.destroy();
1197+
});
1198+
}
1199+
});
1200+
1201+
test("it computes interdependent array computed properties", function() {
1202+
var mapped = get(obj, 'mapped');
1203+
1204+
equal(get(obj, 'max'), 3, 'sanity - it properly computes the maximum value');
1205+
equal(userFnCalls, 0, 'observer is not called on initialisation');
1206+
1207+
var calls = 0;
1208+
Ember.addObserver(obj, 'max', function(){ calls++; });
1209+
1210+
Ember.run(function() {
1211+
obj.get('array').pushObject({ v: 5 });
1212+
});
1213+
1214+
equal(get(obj, 'max'), 5, 'maximum value is updated correctly');
1215+
equal(userFnCalls, 1, 'object defined observers fire');
1216+
equal(calls, 1, 'runtime created observers fire');
1217+
});
1218+

0 commit comments

Comments
 (0)