Knockout-Contrib / Knockout-Validation

A validation library for Knockout JS
1.02k stars 380 forks source link

can't bind to a validatedObservable as the root #202

Open drdamour opened 11 years ago

drdamour commented 11 years ago

i'm trying to do validation across an entire VM and have been running into problems somewhat like the guy @ http://stackoverflow.com/questions/13364788/knockout-validation-validatedobservable-group-error did. He found a solution to his problem @ http://jsfiddle.net/CGuW2/6/ but it seems flawed to me because he is doing roughly this:

var viewModel = {
    num1: ko.observable("50").extend({ number: true, min: 0, max: 100 }),
    num2: ko.observable("50").extend({ number: true, min: 0, max: 100 })
};

viewModel.isValidSum = ko.validatedObservable({
                            num1: viewModel.num1,
                            num2: viewModel.num2
                        }).extend({ mustEqual: 100 });

ko.applyBindings(viewModel);

Seemed weird that the VM had a property that was the VM only observed...and reading the answer at https://github.com/ericmbarnard/Knockout-Validation/issues/41 i thought something like this would be the "right" answer:

var viewModel = {
    num1: ko.observable("50").extend({ number: true, min: 0, max: 100 }),
    num2: ko.observable("50").extend({ number: true, min: 0, max: 100 })
};

var vm = ko.validatedObservable(viewModel).extend({ mustEqual: 101 });

ko.applyBindings(vm);

But when i use this with a tweaked View (see http://jsfiddle.net/drdamour/5B8s4/ ) the validation of the individual input boxes works, but the error property can't be bound to, although it is there, verified by a console.log.

Not being able to use a validatedObservable as a root VM seems like a bug that isn't intended. Is it intended that you can use extend on a validatedObservable to do VM level validation?

stevegreatrex commented 11 years ago

I can't speak for the original intention, but I think that it makes sense to be able to use extend as you have tried there.

I imagine that your example is a dummy one, but what I would suggest in that example is something like this jsFiddle.

Generally you should use the validationMessage binding to display a validation message for a specific observable (instead of binding to error) but that causes an exception when it tries to call extend on the unwrapped view model object (see source). I'm not sure what the appropriate action should be here - maybe check to see if the result is observable before extending, and then extending valueAccessor if not?

The problem you are seeing could also potentially be solved by fixing #173 as that would allow you to bind directly to the error message.

drdamour commented 11 years ago

@stevegreatrex indeed, you have helped me a lot! i think i can better define the problem now.

first, regarding your fiddle, that is exactly the path i was playing with. extending a computed for validation http://jsfiddle.net/drdamour/378mb/ we came to the same answer. but yes, this is a dummy, the validation i need to do cannot key off a computed observable as it's significantly more complex and comes from another library and returns error messages that i want to display.

second, yes i didn't understand the validationMessage binding fully, now i do. thanks much for that, exactly what i'm looking for.

now as to the problem, it really does appear that the problem is limited to using a validatedObservable as the root. If i just warp it with a fake vm it works fine see http://jsfiddle.net/drdamour/WUdLM/

but if you use it as the root it does not work as "validationMessage: $data" biding doesn't work: the message doesn't display and i get the extends method error, see http://jsfiddle.net/drdamour/Vvvqw/

this is exactly what you described because it's been unwrapped.

I can work around this issue for now, where i'll get hung up is if i need to have a collection of validatedObservables with each individual entry needing it's own validation. I don't know how i would display the error messages for each entry. I tried a few things IE:

<div data-bind="foreach: Collection">
...UI for entry
<span data-bind="validationMessage: $data"></span>  <----won't work
<span data-bind="validationMessage: $parent[$index]"></span>  <----won't work
</div>

thanks for linking to the source, it makes it clear i'm not crazy.

drdamour commented 11 years ago

ah, figured a work around. i was surprised to find that $parent within a foreach context isn't the array containing the current iteration..instead it's the object that has the Collection as a member. Based on this i can use this binding to get to the "wrapped" observable:

<span data-bind="validationMessage: $parent.Collection()[$index()]"></span>

not ideal but it works