-
Notifications
You must be signed in to change notification settings - Fork 80
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
Doesn't conform to the spec #14
Comments
'Valid' is one thing, but 'sane' is another. |
@jamen while that would be ideal, in the case of email the accepted use of email syntax has drifted from the spec to a simplified version. For example, try using Gmail to send an email to one of the addresses listed above - I receive an error. |
I don't see "doesn't conform to the spec" as a reasonable issue for this module. I'd suggest that the real problem is that it doesn't document the level of validation it performs, so users of the module have no particular basis for their expectations of the module's behavior. All we have to go on at a glance is "pretty robust" and "only checks form, not function". The examples in |
The library is called "email-validator" - that implies to me that it should assert validity. To me being truly "valid" means exact and correct in every formal variation. The first post listed several addresses that are valid but that this check fails - that is not a validator, so either the code is wrong or the name is wrong. Maybe "email-approximator" or "email-(ad-hoc-informal-subset-of-valid)ator". |
Right. To you, that's what it means, having read just the name of the library. |
When I created this module over 4 years ago, I did it just to quickly validate an e-mail and went with the "good enough" approach for the project I was using it for. I see the merit in providing a full spec validation method, whether that's an additional method/option or just baked in. In any case, I don't currently have any interest in working on improving this myself. I'm happy to merge any pull requests that come in and if anyone would like to take over the project I can transfer the GitHub repo and NPM module. |
Is it sane not to validate an email that finishes with a whitespace ? |
@Meshredded what about trimming the input before validation? ;) |
this is not okay. |
There's a surprising number of responses in projects like this and related StackOverflow questions where users point out that "it doesn't validate For the former, the spec is pretty open and I haven't found a situation where I wanted to accept an email that conformed to edge cases. I don't write things like SMTP libraries, though. For the latter, I'm not interested (e.g.) in sending ecommerce receipts to |
@charlie-s I had a customer once complain that It's just better to conform to the spec all the way, filter out the most egregious email strings which are certainly don't conform to the spec ( I've chosen https://www.npmjs.com/package/isemail since 6 days ago and am not looking back. To be honest, this package does validate |
I would also complain if that address wasn't accepted – I'm not suggesting it shouldn't be, quite the opposite. If you conform to the spec all the way, then you'd allow I'm also using https://www.npmjs.com/package/isemail but with the tld white/black list options to prevent I'm just kind of confused when I stumble upon these threads and inevitably find people wanting |
Fair enough for your constraints, totally an appropriate approach if you send emails asynchronously. In my case I err on the side of correctness over bandwidth. Also, I concede that |
These are valid, but unhandled, e-mail addresses:
See this link for more details:
http://haacked.com/archive/2007/08/21/i-knew-how-to-validate-an-email-address-until-i.aspx/
The text was updated successfully, but these errors were encountered: