New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better validation failure handling #26
Comments
I completely agree with you. Take a good look at this patch: http://github.com/documentcloud/backbone/commit/a09bcbca9d660b09dd010e6fc92c5c6c32f10bbb Ideally, you add an
Sound like a plan? |
So...this means that #bind has a different signature for validation errors? The callback has an eventName as an argument, unless it's a validation error and then has model, error as an argument? I'm a little skeptical about that...I guess I was expecting one of the two:
Anything, so long as it's "backwards compatible" with the existing bind method. Changing signature by event type is a bad idea! I still sort of prefer setting an error property on the model itself, but the solution you've provided is sufficient as well. The error property way is a little simpler, IMO, but with the downside of polluting the model object. You'd only ever check it if save or set failed (returned false) anyway, so I don't see any actual /problems/. Lastly, why is the model passed into the error callback in the |
Nope, the signatures should be roughly the same (the error objects may differ):
Is that better? I agree that it's slightly more verbose than your solution, but I don't think that setting a magic property that sticks around after the fact to clutter things up is really an acceptable API for this. Models are the first argument to error callbacks because you may have a generic error handler that's not bound to a specific model... |
Alright, I'll buy your model-should-be-passed-in argument :) What I meant about differing callback signatures was now you have Do we want to say "the arguments to the callback depend on the event"? |
The arguments to the callback do get passed depending not just on the event, but how you trigger it. Any subsequent arguments to However, in terms of built-in Backbone events, the argument signatures are relatively similar.
|
Hmm, okay. I withdraw my statement then :) |
I still prefer setting a top-level attribute on the model instance. For example, an extra attribute |
An |
Right now, when you save a model, the only way to display a validation error is to bind an error handler on the model object. This is inconvenient, to say the least. If I'm able to modify an instance of a model in two places on the page, with two views...I wouldn't want both views to show the error whenever an error occurs. Nor do I want to bind/unbind error handlers on the model object whenever I switch contexts between the two views.
In my local version of backbone.js, I've added a simple
this.error = error
just above thethis.trigger('error', this, error);
line. This allows me to do something like thisif(!my_model.save()){
alert(my_model.error);
}
Which, with the error binding way, would be rather difficult. Thoughts?
The text was updated successfully, but these errors were encountered: