Skip to content
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

Do not change original data by kv.group #465

Merged
merged 1 commit into from
Nov 24, 2014
Merged

Do not change original data by kv.group #465

merged 1 commit into from
Nov 24, 2014

Conversation

khakulov
Copy link
Contributor

Do not patch isValid, errors and isAnyMessageShown to original given data

ko.validation.init();

var data = {
    first_name: ko.observable('John').extend({required: true}),
    last_name: ko.observable('Doe').extend({required: true}),
    email: ko.observable('john.doe(at)example.com').extend({required: true, email: true}),
};
var validatedObservable = ko.validatedObservable(data);
var normalObservable = ko.observable(data);

validatedObservable.isValid();
validatedObservable.errors();
validatedObservable.errors.showAllMessages();
validatedObservable.errors.isAnyMessageShown();
ko.toJSON(validatedObservable()); // {first_name: "John", last_name: "Doe", email: "john.doe(at)example.com"}
console.log(ko.toJSON(validatedObservable()) === ko.toJSON(data)); // true
console.log(ko.toJSON(normalObservable()) === ko.toJSON(data)); // true
console.log(ko.toJSON(validatedObservable()) === ko.toJSON(normalObservable())); // true

removed

validatedObservable().isValid();
validatedObservable().errors();
validatedObservable().isAnyMessageShown();
ko.toJSON(validatedObservable()); // {"first_name":"John","last_name":"Doe","email":"john.doe(at)example.com","errors":["Please enter a proper email address."]}

@khakulov
Copy link
Contributor Author

My initial Problem was inconsistency of isAnyMessageShown and showAllMessages. They was in different places.

Only difference of validatedObservables and knockout observables should be errors and isValid properties of observable himself.

Whatever all tests are passed so it should be ok. (except isAnyMessageShown, which i moved to errors)

@crissdev crissdev added the idea label Nov 18, 2014
@crissdev
Copy link
Member

Thanks for working on this Azim. I did some tests with this change - on a site that's using this library intensively - and I saw no issues. It seems that usage of methods on validatedObservable() is not very likely to happen - at least in my case.
@stevegreatrex Any thoughs on this before merging it?

@mikocot
Copy link

mikocot commented Jul 29, 2015

In fact our (pretty big) website was extensively using this approach (of calling isValid() on the viewModel itself). It's pretty convenient in some cases. This change basically prevents us from migration to 2.x, but I can understand the reason. Would be best to have a configuration option for this, though.

Cheers

@crissdev
Copy link
Member

crissdev commented Aug 1, 2015

@mikocot As specified in #469 there are some breaking changes but there are ways you can work around them. For this special case you could overwrite the library function - create a proxy for it - and attach the desired methods to the view model provided.

ko.validation.group = (function(originalFunction) {
  return function(obj, options) {
    var result = originalFunction.call(this, obj, options);
    obj.isValid = function() {
      return result().length === 0;
    };
    return result;
  }
})(ko.validation.group);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants