Skip to content

Commit

Permalink
making compute only update itself once on a replace / splice
Browse files Browse the repository at this point in the history
  • Loading branch information
justinbmeyer committed Dec 7, 2012
1 parent cc918fc commit 9cb47df
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 8 deletions.
23 changes: 23 additions & 0 deletions observe/compute/compute_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,27 @@ test("Compute emits change events when an embbedded observe has properties added
obs.removeAttr('bar');
obs.removeAttr('bar');
});

test("compute only updates once when a list's contents are replaced",function(){

var list = new can.Observe.List([{name: "Justin"}]),
computedCount = 0;
var compute = can.compute(function(){
computedCount++;
list.each(function(item){
item.attr('name')
})
})
equals(0,computedCount, "computes are not called until their value is read")
compute.bind("change", function(ev, newVal, oldVal){

})

equals(1,computedCount, "binding computes to store the value");
list.replace([{name: "hank"}]);
equals(2,computedCount, "only one compute")

})


})();
23 changes: 15 additions & 8 deletions observe/observe.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,9 @@ steal('can/util','can/construct', function(can) {
_changes: function(ev, attr, how,newVal, oldVal){
Observe.triggerBatch(this, {type:attr, batchNum: ev.batchNum}, [newVal,oldVal]);
},
_triggerChange: function(attr, how,newVal, oldVal){
Observe.triggerBatch(this,"change",can.makeArray(arguments))
},
/**
* Get or set an attribute or attributes on the observe.
*
Expand Down Expand Up @@ -575,7 +578,7 @@ steal('can/util','can/construct', function(can) {
}
// Let others know the number of keys have changed
Observe.triggerBatch(this, "__keys");
Observe.triggerBatch(this, "change", [prop, "remove", undefined, current]);
this._triggerChange(prop, "remove", undefined, current);

}
return current;
Expand Down Expand Up @@ -659,7 +662,7 @@ steal('can/util','can/construct', function(can) {

}
// `batchTrigger` the change event.
Observe.triggerBatch(this, "change", [prop, changeType, value, current]);
this._triggerChange(prop, changeType, value, current);

//Observe.triggerBatch(this, prop, [value, current]);
// If we can stop listening to our old value, do it.
Expand Down Expand Up @@ -1004,7 +1007,9 @@ steal('can/util','can/construct', function(can) {
can.extend(this, options);
delete this._init;
},
_changes : function(ev, attr, how, newVal, oldVal){
_triggerChange: function(attr, how, newVal, oldVal){

Observe.prototype._triggerChange.apply(this,arguments)
// `batchTrigger` direct add and remove events...
if ( !~ attr.indexOf('.')){

Expand All @@ -1019,7 +1024,7 @@ steal('can/util','can/construct', function(can) {
}

}
Observe.prototype._changes.apply(this,arguments)

},
__get : function(attr){
return attr ? this[attr] : this;
Expand Down Expand Up @@ -1116,13 +1121,15 @@ steal('can/util','can/construct', function(can) {
howMany = args[1] = this.length - index;
}
var removed = splice.apply(this, args);
can.Observe.startBatch()
if ( howMany > 0 ) {
Observe.triggerBatch(this, "change", [""+index, "remove", undefined, removed]);
this._triggerChange(""+index, "remove", undefined, removed);
unhookup(removed, this._cid);
}
if ( args.length > 2 ) {
Observe.triggerBatch(this, "change", [""+index, "add", args.slice(2), removed]);
this._triggerChange(""+index, "add", args.slice(2), removed);
}
can.Observe.stopBatch();
return removed;
},
/**
Expand Down Expand Up @@ -1348,7 +1355,7 @@ steal('can/util','can/construct', function(can) {
res = orig.apply(this, args);

if ( !this.comparator || !args.length ) {
Observe.triggerBatch(this, "change", [""+len, "add", args, undefined])
this._triggerChange(""+len, "add", args, undefined);
}

return res;
Expand Down Expand Up @@ -1412,7 +1419,7 @@ steal('can/util','can/construct', function(can) {
// `undefined` - The new values (there are none).
// `res` - The old, removed values (should these be unbound).
// `len` - Where these items were removed.
Observe.triggerBatch(this, "change", [""+len, "remove", undefined, [res]])
this._triggerChange(""+len, "remove", undefined, [res])

if ( res && res.unbind ) {
res.unbind("change" + this._cid)
Expand Down

2 comments on commit 9cb47df

@scorphus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Justin!

This fixes the issue illustrated by this fiddle: http://jsfiddle.net/scorphus/NbmP7/

@scorphus
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah... to see the issue, activate the log then:

Clear the log; change the category; check the log; repeat until you see double log outputs.

Please sign in to comment.