search

aurelia / validatejs

Enables expressive validation using decorators and/or a fluent API.
MIT License
22 stars 23 forks source link

Validating properties (not fields) #21

Closed MaximBalaganskiy closed 8 years ago

MaximBalaganskiy commented 8 years ago

Putting decorators on a property replaces a getter and a setter

EisenbergEffect commented 8 years ago

@PWKad Adding the the validate decorator should probably detect a getter and setter and wrap them in the new getter/setter rather than generating the "private" backing field. If there's only a getter and no setter or only a setter and no getter, it should probably throw and error, as that would be non-sensical. We will need to add some docs that indicate that in this case we can only guarantee that the rules are evaluated if the setter is the only way to update the property. Updates to any private backing fields or via other mechanisms would be ignored since they can't be detected. The recommendation should be for people to create simple properties though as a general practice.

plwalters commented 8 years ago

Sounds good.

plwalters commented 8 years ago

@jdanyow This should be the lone issue that I could use a big hand on now that I think I've got the getContextFor issue working.

https://github.com/aurelia/validatejs/blob/master/src/property-observer.js

observeProperty currently works by re-writing it with a getter, setter, and a private backing field. Do you have recommendations on how we can do this more sophisticatedly? Code samples should be enough, I tried the above method of wrapping the getter and the setter but it was a rabbit hole of TypeScript vs Babel that kept delivering inconsistent results for me in the timing of when things happened blah blah blah tl:dr help appreciated :)

plwalters commented 8 years ago

https://github.com/aurelia/validatejs/blob/master/src/property-observer.js#L36 <~~ is really all we need to occur for now if the property changes.

jdanyow commented 8 years ago

I think this is the part where I was wondering if we could remove observation from the decorator and make the decorator more of a "rule definer". Then we'd need a service of some sort that could be given instances and their rulesets and it would use this information to setup observation as-needed. I think this would make the validation module more useful/performant in non-ui scenarios or in scenarios where validation on every prop change isn't required. I think the original version of aurelia-validation had some throttling capabilities- this change would improve that scenario as well. Does all this sound good?

EisenbergEffect commented 8 years ago

Makes sense to me. I think it would be best to remove the observation from the decorator and make it more of a metadata piece.

plwalters commented 8 years ago

@jdanyow Sorry just saw this, sure sounds good!

jdanyow commented 8 years ago

fixed in the alpha: http://blog.durandal.io/2016/06/14/new-validation-alpha-is-here/