Chore: Centralize email validation functionality#23816
Conversation
lib/emailValidator.ts
Outdated
| case 'basic': | ||
| return basicEmailRegex.test(email); | ||
| case 'rfc': | ||
| return rfcEmailRegex.test(email); | ||
| default: | ||
| return basicEmailRegex.test(email); |
There was a problem hiding this comment.
| case 'basic': | |
| return basicEmailRegex.test(email); | |
| case 'rfc': | |
| return rfcEmailRegex.test(email); | |
| default: | |
| return basicEmailRegex.test(email); | |
| case 'rfc': | |
| return rfcEmailRegex.test(email); | |
| case 'basic': | |
| default: | |
| return basicEmailRegex.test(email); |
There was a problem hiding this comment.
I thought we had the no-fallthrough rule enabled 🤔
lib/emailValidator.ts
Outdated
| @@ -0,0 +1,13 @@ | |||
| export const validateEmail = (email: string, options: { style: string } = { style: 'basic' }): boolean => { | |||
| const basicEmailRegex = /^.+@.+$/; | |||
There was a problem hiding this comment.
looking into some regex you modified to use this function I found this one interesting: [^@].*@[^@], it makes sure only a single @ is allowed.. wdyt? 👀
There was a problem hiding this comment.
Fair enough, let me do the change 👀
There was a problem hiding this comment.
Not sure 🙈 if we should use it tho
There was a problem hiding this comment.
oopsie.. looks like I missread the regex 🙈
There was a problem hiding this comment.
ok, this one 😬 ^[^@]+@[^@]+$
There was a problem hiding this comment.
That works way better 👀
|
Although I have approved it already, what about writing some unit tests for it? maybe we can have an agreement that all functions in |
|
|
4 participants
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments