skeggse / isemail

validate an email address according to RFCs 5321, 5322, and others
Other
258 stars 43 forks source link

Restructure API #4

Open skeggse opened 9 years ago

skeggse commented 9 years ago

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:

bf commented 8 years ago

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.

nonplus commented 8 years ago

+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);
            }
        }
    });
}
enko commented 6 years ago

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

skeggse commented 6 years ago

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.

alexsasharegan commented 6 years ago

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.

skeggse commented 6 years ago

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.

alexsasharegan commented 6 years ago

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.

hueniverse commented 5 years ago

Is this still happening?

skeggse commented 5 years ago

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.

hueniverse commented 5 years ago

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

skeggse commented 5 years ago

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.

hueniverse commented 5 years ago

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

skeggse commented 5 years ago

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.

hueniverse commented 5 years ago

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?

skeggse commented 5 years ago

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.