search

theironcook / Backbone.ModelBinder

Simple, flexible and powerful Model-View binding for Backbone.
1.42k stars 159 forks source link

Bindings and failed validation #201

Closed platinumazure closed 6 years ago

platinumazure commented 9 years ago

Fixes #178. Note that this is potentially a breaking change.

The basic idea is that if changing the view causes a model set, and that model set fails (ostensibly due to validation errors), the previous model attribute should be copied back over into the view so that the model and view are back in sync.

If this is deemed to be too much of a breaking change, I can augment the pull request to support a new ModelBinder option to enable this behavior (e.g., revertViewOnValidationError or something similar). Just let me know.

Patch impact:

If this change is rejected outright (even with a ModelBinder option), I'll probably create a new pull request with just the test file (minus the last assertion), so that we have some coverage around ModelBinder plus validation. Presumably that would only need a minor version update.

amakhrov commented 9 years ago

Strictly speaking you're right - that's a breaking change. However, I'm not sure that {validate:true} was usable at all with the previous behavior, as it eventually ended up with inconsistent state of view vs model.

Still it's safer to have it as a configurable option for 1.x (so we can easily upgrade the library in the existing projects), and may be turn it on by default in the (future) 2.x release.

platinumazure commented 9 years ago

@amakhrov Besides @theironcook I don't know who the collaborators and other decision-makers might be. If there are others, care to pull them in?

platinumazure commented 9 years ago

@amakhrov Since you opened the original issue, did you have a chance to see if this fixes your issue? (Ignoring for the moment that it might be a "breaking change")

platinumazure commented 9 years ago

@theironcook Could you take a look at this and weigh in on if my proposal is too much of a "breaking change"? As @amakhrov has said, using ModelBinder with model validation is pretty much useless at this point anyway.