.save() should return a rejected promise object for an invalid model

[yuku]

Currently Model#save returns false if it fails validation.

var model = new MyModel({invalid: 'attribute'});
model.save().done(function() {...});
//=> Uncaught TypeError: Object #<error> has no method 'done' 

It should return a rejected promise object instead of false if it is available. (FYI: Zepto does not provide promise object by default)

In addition, the way to notice whether a fail callback is called because of invalid or error is needed.

model.save()
  .done(function () {
    // success
  })
  .fail(function (model, resp, options) {
    if (options.isInvalid) {
      // invalid
    }
  })
  .fail(function (model, resp, options) {
    if (!options.isInvalid) {
      // error
    }
  });

If this proposal makes sense, I will send a pull request.

[wookiehangover]

Thanks for opening an issue!

Any promise behavior in backbone has always been a by product of using jQuery, having a specific code path to support this feature request opens the door to doing this in a lot of other places, and doesn't keep with the historical stance (#2221, #1567, #1775) on this.

I'm closing this one for now, since promises aren't explicitly accounted for in Backbone.

[rymohr]

Just got bit by this one too. It's a pain when you have a hierarchy of models that need to be saved in series.

[kimjoar]

Maybe it's time for Backbone to explicitly support and use promises, not just indirectly through jQuery. The Zepto support is already quite shaky, which has been discussed several times already, e.g. in #2431, so I don't see a big problem in not having it as a potential replacement anymore.

The current solution -- to rely on jQuery having promises, but not using them for validations etc -- creates a really bad API for all the methods which communicate with the server. I think it would be a far better solution in the long run for Backbone to actually use promises internally. This is quite a big change, so it would probably be targeted at 2.0.0.

Are there any specific reasons why Backbone don't use promises (except for supporting Zepto)?

[rymohr]

I was thinking about monkey patching it with something like this in the meantime:

originalSave = Backbone.Model.prototype.save

Backbone.Model.prototype.save = (attributes, options) ->
  req = originalSave.call(@, attributes, options)

  if req
    # model is valid, return the jqxhr
    req
  else
    # model is invalid, return a promise for consistency
    dfd = new $.Deferred()
    dfd.reject(@, @validationError, options)
    dfd.promise()

[jashkenas]

This does seem like it could potentially provide a nicer and more unified API -- @taka84u9, feel free to send that pull request.

[rymohr]

@taka84u9 not sure if others agree but only suggestion I'd have is to rename the option to isValid instead so it's consistent with the existing model.isValid() method.

[akre54]

I took a first shot at this in #2489, but decided not to include the isValid option since the fail handler has to handle the case of both a failed model validation and an unsuccessful ajax request. It feels a little cleaner to me to do your own validation check if save fails, or add an invalid event listener if you really need the specific errors (and you'd probably want an invalid listener to show the user messages anyway)

model.save()
  .fail(function(jqhxr, status, jqerr) {
    if (!model.isValid()) {
      alert('Alas, the model is invalid.');
    } else {
      alert('The model didnt save to the server because ' + jqerr);
    }
  });

Or alternatively:

model.on('invalid', function(m, errors, options) {
  model.errors = errors;
  view.showErrors(errors);
});
model.errors = [];
model.save()
  .fail(function(jqhxr, status, jqerr) {
    if (!_.isEmpty(model.errors)) {
      alert('Alas, the model is invalid.');
    } else {
      alert('The model didnt save to the server because ' + jqerr);
    }
  });

[nvivo]

This should apply for any method related to sync, not only save.

model.destroy has the same issue.

[jashkenas]

Moving the conversation over there.

[Naddiseo]

Collection.create should also use promises instead of having to pass success/error callbacks into it.