From 29a4359ff7d49611cc46cbb9af41815ae18a2646 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 2 Feb 2012 10:32:34 -0500 Subject: [PATCH 1/2] fixes #915 - nested `'change:attr'` events * `'change'` does not fire without changes * nested `'change:attr'` events are fired * `'change'` is only fired once for nested calls * nested `'change'` events are fired --- backbone.js | 21 +++++++++++++----- test/model.js | 60 +++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/backbone.js b/backbone.js index 3dcf3699..257e3b7e 100644 --- a/backbone.js +++ b/backbone.js @@ -233,15 +233,19 @@ var now = this.attributes; var escaped = this._escapedAttributes; var prev = this._previousAttributes || {}; - var alreadyChanging = this._changing; + var alreadySetting = this._setting; this._changed || (this._changed = {}); - this._changing = true; + this._setting = true; // Update attributes. for (attr in attrs) { val = attrs[attr]; if (!_.isEqual(now[attr], val)) delete escaped[attr]; options.unset ? delete now[attr] : now[attr] = val; + if (this._changing && !_.isEqual(this._changed[attr], val)) { + this.trigger('change:' + attr, this, val, options); + this._moreChanges = true; + } delete this._changed[attr]; if (!_.isEqual(prev[attr], val) || (_.has(now, attr) != _.has(prev, attr))) { this._changed[attr] = val; @@ -249,9 +253,9 @@ } // Fire the `"change"` events, if the model has been changed. - if (!alreadyChanging) { + if (!alreadySetting) { if (!options.silent && this.hasChanged()) this.change(options); - this._changing = false; + this._setting = false; } return this; }, @@ -379,12 +383,19 @@ // a `"change:attribute"` event for each changed attribute. // Calling this will cause all objects observing the model to update. change: function(options) { + if (this._changing || !this.hasChanged()) return this; + this._changing = true; for (var attr in this._changed) { this.trigger('change:' + attr, this, this._changed[attr], options); } - this.trigger('change', this, options); + do { + this._moreChanges = false; + this.trigger('change', this, options); + } while (this._moreChanges); this._previousAttributes = _.clone(this.attributes); delete this._changed; + this._changing = false; + return this; }, // Determine if the model has changed since the last `"change"` event. diff --git a/test/model.js b/test/model.js index 3ee29425..346f2f09 100644 --- a/test/model.js +++ b/test/model.js @@ -506,17 +506,6 @@ $(document).ready(function() { a.set({state: 'hello'}); }); - test("Model: Multiple nested calls to set", function() { - var counter = 0, model = new Backbone.Model({}); - model.on('change', function() { - counter++; - model.set({b: 1}); - model.set({a: 1}); - }) - .set({a: 1}); - equal(counter, 1, 'change is only triggered once'); - }); - test("hasChanged/set should use same comparison", function() { expect(2); var changed = 0, model = new Backbone.Model({a: null}); @@ -627,4 +616,53 @@ $(document).ready(function() { equal(changed, 1); }); + test("nested `set` during `'change:attr'`", 1, function() { + var model = new Backbone.Model(); + model.on('change:x', function() { ok(true); }); + model.on('change:y', function() { + model.set({x: true}); + // only fires once + model.set({x: true}); + }); + model.set({y: true}); + }); + + test("nested `change` only fires once", 1, function() { + var model = new Backbone.Model(); + model.on('change', function() { + ok(true); + model.change(); + }); + model.set({x: true}); + }); + + test("no `'change'` event if no changes", function() { + var model = new Backbone.Model(); + model.on('change', function() { ok(false); }); + model.change(); + }); + + test("nested `set` suring `'change'`", 3, function() { + var count = 0; + var model = new Backbone.Model(); + model.on('change', function() { + switch(count++) { + case 0: + deepEqual(this.changedAttributes(), {x: true}); + model.set({y: true}); + break; + case 1: + deepEqual(this.changedAttributes(), {x: true, y: true}); + model.set({z: true}); + break; + case 2: + deepEqual(this.changedAttributes(), {x: true, y: true, z: true}); + break; + default: + ok(false); + } + }); + model.set({x: true}); + }); + }); From 17d0f12a591ac9a3b49284e9c5312009a7db4bc8 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 2 Feb 2012 11:24:39 -0500 Subject: [PATCH 2/2] use a while loop instead of do...while --- backbone.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/backbone.js b/backbone.js index 257e3b7e..ceb22e5b 100644 --- a/backbone.js +++ b/backbone.js @@ -385,13 +385,14 @@ change: function(options) { if (this._changing || !this.hasChanged()) return this; this._changing = true; + this._moreChanges = true; for (var attr in this._changed) { this.trigger('change:' + attr, this, this._changed[attr], options); } - do { + while (this._moreChanges) { this._moreChanges = false; this.trigger('change', this, options); - } while (this._moreChanges); + } this._previousAttributes = _.clone(this.attributes); delete this._changed; this._changing = false;