isemail icon indicating copy to clipboard operation
isemail copied to clipboard

Restructure API

Open skeggse opened this issue 10 years ago • 15 comments

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'}

skeggse avatar Sep 14 '15 17:09 skeggse

Also it'd be great if the callback would conform to the (err, isValid) style. Right now it only uses callback in a callback(isValid) fashion.

bf avatar Jun 03 '16 09:06 bf

+1 on having the callback follow node conventions function(err, result)

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);
            }
        }
    });
}

nonplus avatar Jun 09 '16 21:06 nonplus

Also supporting promises would be nice if checkDNS is set and no callback is specified.

Loopback handles this quite nice:

https://github.com/strongloop/loopback/blob/master/common/models/role.js#L238 https://github.com/strongloop/loopback/blob/master/lib/utils.js#L16

enko avatar Jan 31 '18 07:01 enko

FYI, we no longer implement checkDNS in isemail.

That use-case will be filled via the parse method, whereby users of the library can run their own DNS query against the identified, normalized domain.

skeggse avatar Jan 31 '18 07:01 skeggse

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.

alexsasharegan avatar Feb 01 '18 21:02 alexsasharegan

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.

skeggse avatar Feb 01 '18 21:02 skeggse

Well then let's just stick to adding typings to the existing API. The addition is a single index.d.ts file along with a package.json addition for typings.

I think I've got the API documented. PR #164 is up, @skeggse.

alexsasharegan avatar Feb 01 '18 21:02 alexsasharegan

Is this still happening?

hueniverse avatar Nov 11 '18 08:11 hueniverse

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.

skeggse avatar Nov 11 '18 09:11 skeggse

Is there an api to validate just the domain part of the email?

hueniverse avatar Nov 11 '18 10:11 hueniverse

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.

skeggse avatar Nov 11 '18 18:11 skeggse

No I mean, use the parser to validate a domain, not an email.

hueniverse avatar Nov 11 '18 18:11 hueniverse

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.

skeggse avatar Nov 11 '18 20:11 skeggse

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?

hueniverse avatar Nov 12 '18 06:11 hueniverse

It's mostly that we support domain literals like eran@[127.0.0.1] which isn't useful for domain validation. That said, it could be useful to consolidate unicode-aware domain validation.

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.

skeggse avatar Nov 19 '18 05:11 skeggse