-
Notifications
You must be signed in to change notification settings - Fork 176
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
Allow empty values to pass regex validation #241
base: master
Are you sure you want to change the base?
Conversation
In regex validators (such as number) when the value is undefined (or null) the validator should return success, to comply with the html5 number input. Since this is a breaking change, it will be enabled only using an opt-in flag called "allowEmptyValues".
Sorry for the late review here.
Why this is not decided by the customize RegExp? |
I thought of it, and eventually decided that it is not the RegExp's job to allow empty values: if a user defines a "number" regex, it should really check for number. |
then you can use function(value){
if ( typeof value undefined ) return true;
if ( /* RegExp */ ) return true/false;
} Is this fix your case? |
One more issue about it, if you set |
Do you mean I can use a function as a custom rule? If so, then I guess it's possible but then I won't be able to use all the other built it rules, such as "number"... The basic idea of this PR is to allow undefined values to always pass validation, regardless of which rule is used. The way it is now, if you use "number" rule - the value is automatically required, which I think is wrong. Note that this is also how the default html5 validators work: if you use a "number" input, both |
yes, and you have to update the
Good to see more insight on the 😄 just one more thing, the |
If you are using the Function for the function(value){
if ( typeof value undefined ) return true;
return /* RegExp */;
} If you want to have the |
I see what you mean, and I guess it is possible to create all those rules and the function you've suggested, but it seems a bit like code duplication:
As you can see, the "undefined" check repeats itself for all the rules (except required). It also makes the code longer as we can't use the nice short form of regex, the way it is used now in the included rules. What do you think? Is there another way to make all the rules "optional" without code duplication, and without this PR? |
@liorch88 Thanks for the insight and it perfectly makes sense. maybe we can have one more config when defining it as number: {
RegExp : /* RegExp */,
isRequired : false
} what do you see about this option? I am voting for the below solution. 👇 |
for now, you can // in your angular-validation-rule.js
var RegExpNotRequired = function( regexp ) {
return function (value) {
if (typeof value undefined ) return true;
return /* regexp expression */
}
}
expression = {
required: function(value) {
return !!value;
},
email: RegExpNotRequired( /* email regex */ ),
number: RegExpNotRequired( /* number regex */ ),
url: RegExpNotRequired( /* url regex */ ),
.... |
I guess that would be a nice option too. what would happen to the shorthand regex form that is used in the default rules?
Is it required in that case? (the way it is now) or optional?
Yes that's a nice idea also. it would prevent the duplication in most cases, unless some user decided to split his rules among multiple files. in that case he will have to place that |
It depends on what regexp you are using. Ok, then I should add this solution to Documentation. |
In regex validators (such as number) when the value is undefined (or null) the validator should return success, to comply with the html5 number input.
Since this is a breaking change, it will be enabled only using an opt-in flag called "allowEmptyValues".
I updated existing tests to check when the flag is off (default) and added new tests to check for when its on.