-
Notifications
You must be signed in to change notification settings - Fork 43
Restructure API #4
Comments
Also it'd be great if the callback would conform to the |
+1 on having the callback follow node conventions FYI, this is how I've normalized the API for my usage to make it Node-friendly: /**
* Validate email address for syntax and (optionally) valid MX record
*
* @param {String} email Email address to check
*
* @param {Object} [options] Configuration options
* @param {Boolean} [options.dns=false] check DNS for MX record
* @param {Number} [options.errorLevel=0] strictness of email syntax checking
*
* @param {Function} [callback] Callback function
* @param {Error} callback.err Error if validation fails, otherwise null
* @param {Number} callback.result Diagnostic code
*/
function validateEmail(email, options, callback) {
options = options || {};
if (typeof options === 'function') {
callback = options;
options = {};
}
if (typeof callback !== 'function') {
if (options.checkDNS) {
throw new TypeError('expected callback function for checkDNS option');
}
callback = null;
}
// Convert options to format expected by isemail.validate()
var opts = {
checkDNS: options.dns,
errorLevel: true
};
return isemail.validate(email, opts, function(result) {
if (callback) {
if (result <= (options.errorLevel||0)) {
callback(null, result);
} else {
callback(new Error("Invalid email"), result);
}
}
});
} |
Also supporting promises would be nice if Loopback handles this quite nice: https://github.com/strongloop/loopback/blob/master/common/models/role.js#L238 |
FYI, we no longer implement That use-case will be filled via the |
Some typings would be very appreciated as well! I'm working on annotating the current api to make my project happy, I would be happy to PR my definitions. I'm reading that you're interested in updating the API. If you're open to typescript, perhaps it would be nice to open a PR on a dev branch just defining a TS interface? Not everyone's favorite, but could be a nice way to define the API changes before implementation work. |
I have no idea what that would entail - is it just the addition of a types file? By open dev branch do you mean somehow modifying branch-level permissions? I'd be fine supporting typescript, but I don't have any bandwidth to do so myself. |
Is this still happening? |
I made some headway within the last month. If I can block out another weekend, I'll have this done. That'll enable folks a clean way to test address validity with DNS checks. |
Is there an api to validate just the domain part of the email? |
Not currently - the API I described is mostly done and just exposes the parsed components of the email address. Either the user or another library could wrap isemail and do a DNS check on the parsed domain part. |
No I mean, use the parser to validate a domain, not an email. |
I wasn't planning on it - isemail's domain validation mostly handles the strange syntacies of email domains and domain literals. Is that a use-case you want? Maybe we should move this to a new issue if so. |
I think it would be useful to be able to validate domains in joi instead of implementing it all over again. Is there a reason why an email domain would require different validation than any other FQDN? |
It's mostly that we support domain literals like The only concern there is that I don't have the bandwidth to mitigate IDN homograph attacks and track the current recommendations. I can design the API around that limitation, though. I'll have more time to think about this in a couple weeks. |
I'm not fond of the current API usage, especially the
errorLevel
option. It would also be nice to support email address normalization (removing comments and condensing other particles). What should the API look like?Ideas:
IsEmail.check('[email protected]', {dns: true|false}, callback)
IsEmail.normalize('test(for testing)@hapijs.com') -> '[email protected]'
IsEmail.parse('test(for testing)@hapijs.com') -> {local: 'test', domain: 'hapijs.com'}
The text was updated successfully, but these errors were encountered: