-
Notifications
You must be signed in to change notification settings - Fork 734
fix: Allow empty strings for email and number validation in validators.ts #2811
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
fix: Allow empty strings for email and number validation in validators.ts #2811
Conversation
…tring Fixes/validator allow emty string
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Thank you! @Inbal-Tish @adids1221 could you please take a peek whenever able 🙏 ? This one has been open for a few months now |
Co-authored-by: Adi Mordo <[email protected]>
Co-authored-by: Adi Mordo <[email protected]>
@@ -8,9 +8,15 @@ const priceRegEx = /^[0-9]{1,9}([.][0-9]{1,2})?$/; | |||
|
|||
const validators = { | |||
required: (value = '') => !_.isEmpty(value), | |||
email: (value = '') => new EmailValidator().isValid(value), | |||
email: (value = '') => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it could be a breaking change for other users.
I think we should add a new validation optionalEmail
that will validate this case (empty string) and leave the email
validator as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left another small comment since it could be a breaking change to other users that are using this validation even though the email
field isn't required.
changes are made as per the suggestion |
Description
Updated the
email
andnumber
validators invalidator.ts
to allow empty strings. Now, empty email and number strings are considered valid.Motivation
issue #2807
Changelog
email
validator logic to allow empty strings explicitly.number
validator logic to consider empty strings as valid.Additional info
This change ensures a more permissive validation for both email and number fields. If a field is not required, no error message will be displayed when users clear their input.
If you encounter any issues or have further suggestions, please let me know.
Results