scttnlsn / backbone.io

Backbone.js sync via Socket.IO
http://scttnlsn.github.io/backbone.io
541 stars 66 forks source link

Fixes for backbone 1.0 compatibility #40

Closed sithmel closed 11 years ago

scttnlsn commented 11 years ago

Why did you remove the model argument from the error and success callbacks?

sithmel commented 11 years ago

Backbone.js documentation shows that method "save" takes an optional callback "success" with model, response, options as arguments. http://backbonejs.org/#Model-save But this callback is not the same called by Backbone.sync. It is wrapped inside another success callback. In the original source code you can see this:

save : function(attrs, options) {
   ...
  var success = options.success; // this is the original "success" callback 
  options.success = function(resp, status, xhr) { // this is the callback called by sync
    if (!model.set(model.parse(resp, xhr), options)) return false;
    if (success) success(model, resp, xhr);
  };
  options.error = wrapError(options.error, model, options);
  var method = this.isNew() ? 'create' : 'update';
  return (this.sync || Backbone.sync).call(this, method, this, options);
},

Watching better Backbone.js history It seems to me that this was an old backbone.io issue and my fix is not even complete. The correct functions signature should be:

success(resp); // status and xhr doesn't make sense in our case

and

error(resp); // see Backbone.wrapError 
sithmel commented 11 years ago

I'll make a better commented, more complete pull request in few days, ok?

scttnlsn commented 11 years ago

Awesome...thank you!