Skip to content

Instantly share code, notes, and snippets.

@mattheworiordan
Created June 21, 2011 14:35
Show Gist options
  • Star 6 You must be signed in to star a gist
  • Fork 7 You must be signed in to fork a gist
  • Save mattheworiordan/1037984 to your computer and use it in GitHub Desktop.
Save mattheworiordan/1037984 to your computer and use it in GitHub Desktop.
Backbone patch to defer update method requests when new create requests are not complete on a model
(function() {
Backbone.Model.prototype._save = Backbone.Model.prototype.save;
Backbone.Model.prototype.save = function(attrs, options) {
var that = this;
if (!options) { options = {}; }
if (this.savingNewRecord) {
// store or replace last PUT request with the latest version, we can safely replace old PUT requests with new ones
// but if there are callbacks from a previous PUT request, we need to make sure they are all called as well
_(['success','error']).each(function(event) {
// convert all callbacks to a single array of callbacks)
var existingCallbacks = that.putQueue ? that.putQueue.options[event] : [];
options[event] = _.compact( existingCallbacks.concat(options[event]) );
});
this.putQueue = { attributes: _.extend({}, that.attributes, attrs), options: options };
} else {
if (this.isNew()) {
// saving a new record so we need to wait for server to respond and assign an ID before the model is saved again
this.savingNewRecord = true;
// store the old callback and overwrite so we can catch the success/error event when savint this model
_(['success','error']).each(function(event) {
var oldEventCallback = options[event];
options[event] = function(model, response) {
that.savingNewRecord = false;
// check if callback for this model save exists and if so execute the callback
if (oldEventCallback) { oldEventCallback.apply(this, model, response); }
// if there is a PUT waiting to fire, fire it now
if (that.putQueue) {
// as PUT builds up callbacks as arrays, lets create a callback which executes all the callbacks
_(['success','error']).each(function(callBackEvent) {
var callbacks = _.clone(that.putQueue.options[callBackEvent]);
that.putQueue.options[callBackEvent] = function(model, response) {
var callbackThis = this;
_(callbacks).each(function(callback) {
callback.apply(callbackThis, model, response);
})
};
});
Backbone.Model.prototype.save.call(that, _.extend({}, that.attributes, that.putQueue.attributes), that.putQueue.options);
}
};
});
}
Backbone.Model.prototype._save.call(this, attrs, options);
}
};
}());
@philfreo
Copy link

(Link to Backbone issue: jashkenas/backbone#345)

Warning: this gist won't work with Backbone-relational because it also implements a "processQueue". Renaming it makes it work though.

@philfreo
Copy link

Also I found that you may want to change line 13 to say if (this.isNew() && this.saving) -- since the main reason to use this script is to prevent multiple POSTs from firing before the first one returns, but after the 1st one returns and an id is set it is okay (sometimes better) if the subsequent PUTs don't queue up / fire in serial.

@mattheworiordan
Copy link
Author

@philfreo I'm not sure I entirely agree that adding this.isNew() to line 13 is the right choice in all cases. If Backbone was smart enough to only send the columns that changed in a PUT request, then this would be the right thing, but because Backbone in fact sends a JSON representation of the entire object, it is quite plausible that two near simultaneous requests get fired out of order meaning your last set of changes are overwritten by the previous request. I understand the motivation for your change, however I personally would not take that chance.

@philfreo
Copy link

Ok, I agree. Then I guess the root of the problem is that I didn't like how PUTs were queueing up so much. Since Backbone sends the entire object, if a PUT is queued and there are other PUTs still in the queue that haven't yet been sent, then shouldn't we drop the former ones and replace it with the latest only? Otherwise if a bunch of near simultaneous save()s get fired, it can take way longer / many in-serial http requests to make all those saves.

@mattheworiordan
Copy link
Author

@philfreo, I agree, that is the right approach. I will make some changes to this Gist shortly and you can tell me what you think.

@mattheworiordan
Copy link
Author

@philfreo, what do you think of this version?

It will only do one PUT request whilst waiting for a isNew() record to save. I've tested it in my environment and it seems to work OK. As you will see, it's pretty much a complete rewrite, and quite robust as even though it compacts all PUT requests into one, it still calls all the callbacks passed in.

There are some risks with this code though:

  • If you explicitly set attributes in a previous PUT request as part of your save call, and then do another save the attributes you passed into your first request may be lost
  • If the POST request updates fields other than ID i.e. say email is changed from matt to matt@gmail.com in the response, this too could be lost
  • If the PUT requests have an explicit value set to trigger an event, that too could be lost as the future PUT requests may not set that value.

All of these situations above are edge cases, and largely existed before this latest version of the Gist, but worth pointing out anyway.

@philfreo
Copy link

Perhaps something else in my app is interferring with its behavior, but I just plugged in the latest version here and it just doesn't work for me (get tons of PUT requests fired immediately)

@philfreo
Copy link

I tried your latest version again after some other cleanup in my app, and I get this:
http://cl.ly/image/2E1P1V3O2V38

@philfreo
Copy link

(which includes some parallel requests -- and also it's queuing up way too many still - later ones don't seem to be canceling earlier ones)

@rinatio
Copy link

rinatio commented Oct 11, 2012

If i use this code $.when(model.save()).done(callback) doesn't work.

@rinatio
Copy link

rinatio commented Oct 11, 2012

And evens don't work: model.on('success', callback)

@rinatio
Copy link

rinatio commented Oct 11, 2012

Here is possible fix, but i'm not sure about events triggering: https://gist.github.com/3873386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment