noahsalvi / svelte-use-form

The most compact reactive form controller (including Validation) that you'll ever see.
MIT License
252 stars 14 forks source link

Email validator may be insufficient? #59

Closed keehun closed 1 year ago

keehun commented 1 year ago

I would love to know where the email validator regex came from.

https://github.com/noahsalvi/svelte-use-form/blob/0f17f7ec26186fb04bc210c8ac3c28e8f527b45e/src/lib/validators.ts#L21

I understand the value of keeping these regexes as small, general, and as simple as possible, but I wonder if there are different regexes that might work better. For example, maybe we can add an email2 validator that enforces the presence of a TLD. The current email validator allows email addresses like test@test or email@localhost. While those may be technically valid email addresses, I want to ensure there is a TLD for my website.

Of course, I could write my own validator, but wondered what your thoughts were on adding an alternate email validator to the library itself.

Thank you for the library! It's been such a great help for me. Thank you.

keehun commented 1 year ago

The following minor modification to the regex in the library forces the presence of a TLD:

[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+
noahsalvi commented 1 year ago

I like your idea of adding it as an alternative validator!

Regarding where that regex came from... probably Stackoverflow but what I still remember from that time is that the official spec also uses a regex which allows emails with no TLD. Which is why I would prefer to keep the original email validator as it is. https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address

However it is true that almost all sites prefer to require a TLD.

Are you interested in adding it and also updating the readme to include it? And Is there a better name for it than email2?

keehun commented 1 year ago

I'd be happy to contribute a PR. I'll probably get that up in the next few days.

keehun commented 1 year ago

Do I need repo write permissions? I'm ready to push up a branch & open a PR, but I can't seem to do that. Should I fork and do it that way instead?

noahsalvi commented 1 year ago

Yes exactly. You'd need to fork it and then create a PR into the branch i just created 59-tld-email-validator