Permalink
Browse files

refactor change/set, fixing #1478 and #1664

  • Loading branch information...
1 parent cb988aa commit 71b6404d73f506627f0cc516177e39cc8c9947d7 @tgriesser tgriesser committed Sep 30, 2012
Showing with 95 additions and 44 deletions.
  1. +53 −42 backbone.js
  2. +42 −2 test/model.js
View
@@ -193,12 +193,12 @@
this._escapedAttributes = {};
this.cid = _.uniqueId('c');
this.changed = {};
- this._silent = {};
+ this._changes = {};
this._pending = {};
this.set(attrs, {silent: true});
// Reset change tracking.
this.changed = {};
- this._silent = {};
+ this._changes = {};
this._pending = {};
this._previousAttributes = _.clone(this.attributes);
this.initialize.apply(this, arguments);
@@ -210,14 +210,18 @@
// A hash of attributes whose current and previous value differ.
changed: null,
- // A hash of attributes that have silently changed since the last time
- // `change` was called. Will become pending attributes on the next call.
- _silent: null,
+ // A hash of attributes that have changed since the last time `change`
+ // was called.
+ _changes: null,
- // A hash of attributes that have changed since the last `'change'` event
+ // A hash of attributes that have changed since the last `change` event
// began.
_pending: null,
+ // A hash of attributes with the current model state to determine if
+ // a `change` should be recorded within a nested `change` block
+ _changing : null,
+
// The default name for the JSON `id` attribute is `"id"`. MongoDB and
// CouchDB users may want to set this to `"_id"`.
idAttribute: 'id',
@@ -257,31 +261,30 @@
// Set a hash of model attributes on the object, firing `"change"` unless
// you choose to silence it.
- set: function(key, value, options) {
- var attrs, attr, val;
+ set: function(attrs, options) {
+ var attr, key, val;
+ if (attrs == null) return this;
// Handle both `"key", value` and `{key: value}` -style arguments.
- if (_.isObject(key) || key == null) {
- attrs = key;
- options = value;
- } else {
- attrs = {};
- attrs[key] = value;
+ if (!_.isObject(attrs)) {
+ key = attrs;
+ (attrs = {})[key] = options;
+ options = arguments[2];
}
// Extract attributes and options.
- options || (options = {});
- if (!attrs) return this;
+ var silent = options && options.silent;
+ var unset = options && options.unset;
if (attrs instanceof Model) attrs = attrs.attributes;
- if (options.unset) for (attr in attrs) attrs[attr] = void 0;
+ if (unset) for (attr in attrs) attrs[attr] = void 0;
// Run validation.
if (!this._validate(attrs, options)) return false;
// Check for changes of `id`.
if (this.idAttribute in attrs) this.id = attrs[this.idAttribute];
- var changes = options.changes = {};
+ var changing = this._changing;
var now = this.attributes;
var escaped = this._escapedAttributes;
var prev = this._previousAttributes || {};
@@ -291,27 +294,30 @@
val = attrs[attr];
// If the new and current value differ, record the change.
- if (!_.isEqual(now[attr], val) || (options.unset && _.has(now, attr))) {
+ if (!_.isEqual(now[attr], val) || (unset && _.has(now, attr))) {
delete escaped[attr];
- (options.silent ? this._silent : changes)[attr] = true;
+ this._changes[attr] = true;
}
// Update or delete the current value.
- options.unset ? delete now[attr] : now[attr] = val;
+ unset ? delete now[attr] : now[attr] = val;
// If the new and previous value differ, record the change. If not,
// then remove changes for this attribute.
if (!_.isEqual(prev[attr], val) || (_.has(now, attr) !== _.has(prev, attr))) {
this.changed[attr] = val;
- if (!options.silent) this._pending[attr] = true;
+ if (!silent) this._pending[attr] = true;
} else {
delete this.changed[attr];
delete this._pending[attr];
+ if (!changing) delete this._changes[attr];
}
+
+ if (changing && _.isEqual(now[attr], changing[attr])) delete this._changes[attr];
}
// Fire the `"change"` events.
- if (!options.silent) this.change(options);
+ if (!silent) this.change(options);
return this;
},
@@ -346,16 +352,14 @@
// Set a hash of model attributes, and sync the model to the server.
// If the server returns an attributes hash that differs, the model's
// state will be `set` again.
- save: function(key, value, options) {
- var attrs, current, done;
+ save: function(attrs, options) {
+ var key, current, done;
- // Handle both `("key", value)` and `({key: value})` -style calls.
- if (_.isObject(key) || key == null) {
- attrs = key;
- options = value;
- } else {
- attrs = {};
- attrs[key] = value;
+ // Handle both `"key", value` and `{key: value}` -style arguments.
+ if (attrs != null && !_.isObject(attrs)) {
+ key = attrs;
+ (attrs = {})[key] = options;
+ options = arguments[2];
}
options = options ? _.clone(options) : {};
@@ -455,18 +459,25 @@
// a `"change:attribute"` event for each changed attribute.
// Calling this will cause all objects observing the model to update.
change: function(options) {
- options || (options = {});
var changing = this._changing;
- this._changing = true;
+ var current = this._changing = {};
// Silent changes become pending changes.
- for (var attr in this._silent) this._pending[attr] = true;
+ for (var attr in this._changes) this._pending[attr] = true;
+
+ // Trigger 'change:attr' for any new or silent changes.
+ var changes = this._changes;
+ this._changes = {};
- // Silent changes are triggered.
- var changes = _.extend({}, options.changes, this._silent);
- this._silent = {};
+ // Set the correct state for this._changing values
+ var triggers = [];
for (var attr in changes) {
- this.trigger('change:' + attr, this, this.get(attr), options);
+ current[attr] = this.get(attr);
+ triggers.push(attr);
+ }
+
+ for (var i=0, l=triggers.length; i < l; i++) {
+ this.trigger('change:' + triggers[i], this, current[triggers[i]], options);
}
if (changing) return this;
@@ -476,13 +487,13 @@
this.trigger('change', this, options);
// Pending and silent changes still remain.
for (var attr in this.changed) {
- if (this._pending[attr] || this._silent[attr]) continue;
+ if (this._pending[attr] || this._changes[attr]) continue;
delete this.changed[attr];
}
this._previousAttributes = _.clone(this.attributes);
}
- this._changing = false;
+ this._changing = null;
return this;
},
@@ -532,7 +543,7 @@
// returning `true` if all is well. If a specific `error` callback has
// been passed, call that instead of firing the general `"error"` event.
_validate: function(attrs, options) {
- if (options.silent || !this.validate) return true;
+ if (options && options.silent || !this.validate) return true;
attrs = _.extend({}, this.attributes, attrs);
var error = this.validate(attrs, options);
if (!error) return true;
View
@@ -740,9 +740,9 @@ $(document).ready(function() {
model.set({b: 2}, {silent: true});
});
model.set({b: 0});
- deepEqual(changes, [0, 1, 1]);
+ deepEqual(changes, [0, 1]);
model.change();
- deepEqual(changes, [0, 1, 1, 2, 1]);
+ deepEqual(changes, [0, 1, 2, 1]);
});
test("nested set multiple times", 1, function() {
@@ -827,4 +827,44 @@ $(document).ready(function() {
var undefinedattrs = new Model(undefined);
});
+ asyncTest("#1478 - Model `save` does not trigger change on unchanged attributes", 0, function() {
+ var Model = Backbone.Model.extend({
+ sync: function(method, model, options) {
+ setTimeout(function(){
+ options.success();
+ start();
+ }, 0);
+ }
+ });
+ new Model({x: true})
+ .on('change:x', function(){ ok(false); })
+ .save(null, {wait: true});
+ });
+
+ test("#1664 - Changing from one value, silently to another, back to original does not trigger change.", 0, function() {
+ var model = new Backbone.Model({x:1});
+ model.on('change:x', function() { ok(false); });
+ model.set({x:2},{silent:true});
+ model.set({x:3},{silent:true});
+ model.set({x:1});
+ });
+
+ test("#1664 - multiple silent changes nested inside a change event", 2, function() {
+ var changes = [];
+ var model = new Backbone.Model();
+ model.on('change', function() {
+ model.set({a:'c'}, {silent:true});
+ model.set({b:2}, {silent:true});
+ model.unset('c', {silent:true});
+ model.set({a:'a'}, {silent:true});
+ model.set({b:1}, {silent:true});
+ model.set({c:'item'}, {silent:true});
+ });
+ model.on('change:a change:b change:c', function(model, val) { changes.push(val); });
+ model.set({a:'a', b:1, c:'item'});
+ deepEqual(changes, ['a',1,'item']);
+ model.change();
+ deepEqual(changes, ['a',1,'item']);
+ });
+
});

0 comments on commit 71b6404

Please sign in to comment.