ansman / validate.js

A declarative validation library written javascript
https://validatejs.org
MIT License
2.63k stars 336 forks source link

isEmpty() instead of isDefined() #219

Open abhirathmahipal opened 7 years ago

abhirathmahipal commented 7 years ago

The inbuilt validation functions like length, format etc check if the attribute is defined or not. presence validator enforces the particular attribute to be a part of the dictionary.

In case a user wants to update a model (Eg cars). He won't be sending all the attributes of that model. Say if he wants to unset the field carOwner and therefore sends null or "". Since it's empty I wouldn't want length validator to validate it. This way validation would not fail and it'll also give the user of the library the option to allow certain empty type values.

The presence validator can be extended to allow various types of empty values.

var constraints = {
    "carOwner": {
        length: {minimum: 1, maximum: 50},
        allowEmpty: [undefined, null, '']   // this allows the user to send null and empty strings but not {} and []
     }
}

I made the said changes on my local machine and I get 7 failing test cases. All of them failed due to '' or ' ' in the attribute field. (this is expected behaviour as the changes I made expect undefined as a result of being empty but the current test cases expect an error message). No other test cases fail and I am strongly inclined to believe that it doesn't break any functionality.

@ansman What are your views on this? (I haven't considered backward compatibility.)

ansman commented 6 years ago

Interesting, I haven't really considered treating null and undefined differently but of course that makes sense.

abhirathmahipal commented 6 years ago

@ansman Check out my fork. I've written test cases for the new functionality and also have updated documentation wherever possible.

I think I've closely followed your coding standards as well. Check it out when you time permits :)