From 3f004196bd5602864bf1fb3f95eea2cd997733d9 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Wed, 9 Nov 2011 14:05:58 -0500 Subject: [PATCH 1/8] assert that change is only triggered once --- test/model.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/model.js b/test/model.js index 4963248b..8b47d571 100644 --- a/test/model.js +++ b/test/model.js @@ -450,12 +450,14 @@ $(document).ready(function() { }); test("Model: Multiple nested calls to set", function() { - var model = new Backbone.Model({}); + var counter = 0, model = new Backbone.Model({}); model.bind('change', function() { + counter++; model.set({b: 1}); model.set({a: 1}); }) .set({a: 1}); + equal(counter, 1, 'change is only triggered once'); }); }); From 368953eb3a9aa882aba085761e149d3ca10db975 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 10 Nov 2011 08:54:59 -0500 Subject: [PATCH 2/8] implement unset/clear in terms of set --- backbone.js | 74 ++++++++++++----------------------------------------- 1 file changed, 16 insertions(+), 58 deletions(-) diff --git a/backbone.js b/backbone.js index d5b01440..6cb805da 100644 --- a/backbone.js +++ b/backbone.js @@ -202,8 +202,8 @@ // Update attributes. for (var attr in attrs) { var val = attrs[attr]; - if (!_.isEqual(now[attr], val)) { - now[attr] = val; + if (!_.isEqual(now[attr], val) || options.unset && (attr in now)) { + options.unset ? delete now[attr] : now[attr] = val; delete escaped[attr]; this._changed = true; if (!options.silent) this.trigger('change:' + attr, this, val, options); @@ -220,53 +220,19 @@ // Remove an attribute from the model, firing `"change"` unless you choose // to silence it. `unset` is a noop if the attribute doesn't exist. - unset : function(attr, options) { - if (!(attr in this.attributes)) return this; - options || (options = {}); - var value = this.attributes[attr]; - - // Run validation. - var validObj = {}; - validObj[attr] = void 0; - if (!options.silent && this.validate && !this._performValidation(validObj, options)) return false; - - // changedAttributes needs to know if an attribute has been unset. - (this._unsetAttributes || (this._unsetAttributes = [])).push(attr); - - // Remove the attribute. - delete this.attributes[attr]; - delete this._escapedAttributes[attr]; - if (attr == this.idAttribute) delete this.id; - this._changed = true; - if (!options.silent) { - this.trigger('change:' + attr, this, void 0, options); - this.change(options); - } - return this; + unset : function(attrs, options) { + var key = attrs; + if (_.isString(key)) (attrs = {})[key] = void 0; + (options || (options = {})).unset = true; + return this.set(attrs, options); }, // Clear all attributes on the model, firing `"change"` unless you choose // to silence it. clear : function(options) { - options || (options = {}); - var attr; - var old = this.attributes; - - // Run validation. - var validObj = {}; - for (attr in old) validObj[attr] = void 0; - if (!options.silent && this.validate && !this._performValidation(validObj, options)) return false; - - this.attributes = {}; - this._escapedAttributes = {}; - this._changed = true; - if (!options.silent) { - for (attr in old) { - this.trigger('change:' + attr, this, void 0, options); - } - this.change(options); - } - return this; + var attrs = {}; + for (var attr in this.attributes) if (attr != this.idAttribute) attrs[attr] = void 0; + return this.unset(attrs); }, // Fetch the model from the server. If the server's representation of the @@ -346,7 +312,6 @@ change : function(options) { this.trigger('change', this, options); this._previousAttributes = _.clone(this.attributes); - this._unsetAttributes = null; this._changed = false; }, @@ -363,22 +328,15 @@ // the server. Unset attributes will be set to undefined. changedAttributes : function(now) { now || (now = this.attributes); - var old = this._previousAttributes, unset = this._unsetAttributes; - - var changed = false; + var changed = false, old = this._previousAttributes; for (var attr in now) { - if (!_.isEqual(old[attr], now[attr])) { - changed || (changed = {}); - changed[attr] = now[attr]; - } + if (_.isEqual(old[attr], now[attr])) continue; + (changed || (changed = {}))[attr] = now[attr]; } - - if (unset) { - changed || (changed = {}); - var len = unset.length; - while (len--) changed[unset[len]] = void 0; + if (!this._changed) return changed; + for (var attr in old) { + if (!(attr in now)) (changed || (changed = {}))[attr] = void 0; } - return changed; }, From 12b11410d0961989b9671185a3b1ab094d551ffa Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 10 Nov 2011 09:59:49 -0500 Subject: [PATCH 3/8] ignore values when unsetting --- backbone.js | 1 + 1 file changed, 1 insertion(+) diff --git a/backbone.js b/backbone.js index 6cb805da..5d906f0c 100644 --- a/backbone.js +++ b/backbone.js @@ -187,6 +187,7 @@ options || (options = {}); if (!attrs) return this; if (attrs.attributes) attrs = attrs.attributes; + if (options.unset) for (var attr in attrs) attrs[attr] = void 0; var now = this.attributes, escaped = this._escapedAttributes; // Run validation. From 66d209d4f9e22fae3b4d835fb6de6432cb58c9ae Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 10 Nov 2011 10:10:17 -0500 Subject: [PATCH 4/8] unit test for ignore values when unsetting --- test/model.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/model.js b/test/model.js index 8b47d571..57780978 100644 --- a/test/model.js +++ b/test/model.js @@ -140,19 +140,24 @@ $(document).ready(function() { }); test("Model: set and unset", function() { + expect(8); attrs = {id: 'id', foo: 1, bar: 2, baz: 3}; a = new Backbone.Model(attrs); var changeCount = 0; a.bind("change:foo", function() { changeCount += 1; }); a.set({'foo': 2}); - ok(a.get('foo')== 2, "Foo should have changed."); + ok(a.get('foo') == 2, "Foo should have changed."); ok(changeCount == 1, "Change count should have incremented."); a.set({'foo': 2}); // set with value that is not new shouldn't fire change event - ok(a.get('foo')== 2, "Foo should NOT have changed, still 2"); + ok(a.get('foo') == 2, "Foo should NOT have changed, still 2"); ok(changeCount == 1, "Change count should NOT have incremented."); - a.unset('foo'); - ok(a.get('foo')== null, "Foo should have changed"); + a.validate = function(attrs) { + ok(attrs.foo === void 0, 'ignore values when unsetting'); + }; + a.unset({foo: 1}); + ok(a.get('foo') == null, "Foo should have changed"); + delete a.validate; ok(changeCount == 2, "Change count should have incremented for unset."); a.unset('id'); From 93ad86cfa9fe5b6aa7b70d6cac232fe40a2879c8 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 10 Nov 2011 13:45:27 -0500 Subject: [PATCH 5/8] add tests for #730 and #565 --- test/model.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/model.js b/test/model.js index 57780978..ac1ab0a9 100644 --- a/test/model.js +++ b/test/model.js @@ -204,11 +204,18 @@ $(document).ready(function() { test("Model: clear", function() { var changed; - var model = new Backbone.Model({name : "Model"}); + var model = new Backbone.Model({id: 1, name : "Model"}); model.bind("change:name", function(){ changed = true; }); + model.bind("change", function() { + var changedAttrs = model.changedAttributes(); + ok('name' in changedAttrs); + ok(!('id' in changedAttrs)); + }); model.clear(); equals(changed, true); equals(model.get('name'), undefined); + equals(model.id, 1); + equals(model.get('id'), 1); }); test("Model: defaults", function() { From 0c08ab8185749bb0ddec4dbafc26db15b4f87457 Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Thu, 10 Nov 2011 13:47:11 -0500 Subject: [PATCH 6/8] bail immediately if !this._changed --- backbone.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backbone.js b/backbone.js index 5d906f0c..3ec4c293 100644 --- a/backbone.js +++ b/backbone.js @@ -328,13 +328,13 @@ // view need to be updated and/or what attributes need to be persisted to // the server. Unset attributes will be set to undefined. changedAttributes : function(now) { + if (!this._changed) return false; now || (now = this.attributes); var changed = false, old = this._previousAttributes; for (var attr in now) { if (_.isEqual(old[attr], now[attr])) continue; (changed || (changed = {}))[attr] = now[attr]; } - if (!this._changed) return changed; for (var attr in old) { if (!(attr in now)) (changed || (changed = {}))[attr] = void 0; } From d87144d6cb4d26c7321d5ad5ebf8f6d7dc4a4e4b Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Fri, 11 Nov 2011 10:55:36 -0500 Subject: [PATCH 7/8] unset accepts var args --- backbone.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/backbone.js b/backbone.js index 3ec4c293..06b444f7 100644 --- a/backbone.js +++ b/backbone.js @@ -222,8 +222,11 @@ // Remove an attribute from the model, firing `"change"` unless you choose // to silence it. `unset` is a noop if the attribute doesn't exist. unset : function(attrs, options) { - var key = attrs; - if (_.isString(key)) (attrs = {})[key] = void 0; + if (_.isString(attrs)) { + var key, args = _.toArray(arguments), attrs = {}; + while (_.isString(key = args.shift())) attrs[key] = void 0; + options = key; + } (options || (options = {})).unset = true; return this.set(attrs, options); }, @@ -231,9 +234,8 @@ // Clear all attributes on the model, firing `"change"` unless you choose // to silence it. clear : function(options) { - var attrs = {}; - for (var attr in this.attributes) if (attr != this.idAttribute) attrs[attr] = void 0; - return this.unset(attrs); + var keys = _.without(_.keys(this.attributes), 'id'); + return this.unset.apply(this, keys.concat([options])); }, // Fetch the model from the server. If the server's representation of the From 7658021e98890ca8e28177cf0f7f8f4f53ea09bc Mon Sep 17 00:00:00 2001 From: Brad Dunbar Date: Wed, 16 Nov 2011 14:13:15 -0500 Subject: [PATCH 8/8] simpler arg parsing --- backbone.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backbone.js b/backbone.js index 06b444f7..84e91c50 100644 --- a/backbone.js +++ b/backbone.js @@ -223,9 +223,8 @@ // to silence it. `unset` is a noop if the attribute doesn't exist. unset : function(attrs, options) { if (_.isString(attrs)) { - var key, args = _.toArray(arguments), attrs = {}; - while (_.isString(key = args.shift())) attrs[key] = void 0; - options = key; + var args = _.toArray(arguments), attrs = {}; + while (_.isString(options = args.shift())) attrs[options] = void 0; } (options || (options = {})).unset = true; return this.set(attrs, options);