Skip to content
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

Implement isemail v4 #200

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement isemail v4 #200

wants to merge 1 commit into from

Conversation

skeggse
Copy link
Owner

@skeggse skeggse commented Feb 8, 2019

Time to deprecate/break things! Also a chance to start adding some tasty new APIs.

Still very work-in-progress. I'll try and resolve #4 (including #4 (comment)) and #162.

New things found while working on this:

Other things found while working on this:

This is just so I can feel a little more sane and spend less time
trying to remember how everything worked. I've spent too much time
starting blankly at code I wrote a few months ago only to realize that
the types were vastly different than what I was remembering.
@skeggse skeggse added this to the 4.0.0 milestone Feb 8, 2019
@skeggse skeggse self-assigned this Feb 8, 2019
@andreialecu
Copy link

andreialecu commented Apr 23, 2021

Was wondering if this PR was abandoned. It seems so.

TypeScript may be a better idea nowadays than using Flow.

It appears there's no good email validation library in the ecosystem that works on node, browsers and react native. At least not one that doesn't use regex: https://davidcel.is/posts/stop-validating-email-addresses-with-regex/

The only thing blocking isemail from working in a browser or react native is the util import here:

const Util = require('util');

Additionally, there are some usages of Buffer.byteLength, and react native does not have Buffer so it needs to be polyfilled.

I could come up with a PR that fixes both issues, but I was wondering if it's worth doing it on the current master or if this 4.0 rewrite is still planned.

In the mean time I'm using a quick and dirty patch, and a Buffer polyfill (no need for the cross-context type checking for my purposes):

diff --git a/lib/index.js b/lib/index.js
index 4f3d4cc33d13e68476dbe9ff0021081458af3291..14c2d9a2e83cc0f973f5883030b0e6c34644fe53 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -3,7 +3,6 @@
 // Load modules
 
 const Punycode = require('punycode');
-const Util = require('util');
 
 // Declare internals
 
@@ -219,10 +218,8 @@ if (typeof Symbol !== 'undefined') {
 // Node 10 introduced isSet and isMap, which are useful for cross-context type
 // checking.
 // $lab:coverage:off$
-internals._isSet = (value) => value instanceof Set;
-internals._isMap = (value) => value instanceof Map;
-internals.isSet = Util.types && Util.types.isSet || internals._isSet;
-internals.isMap = Util.types && Util.types.isMap || internals._isMap;
+internals.isSet = (value) => value instanceof Set;
+internals.isMap = (value) => value instanceof Map;
 // $lab:coverage:on$

(related: #157)

@skeggse
Copy link
Owner Author

skeggse commented Apr 23, 2021

I don't have bandwidth for this project (v4 or otherwise), or a real reason to keep working on it. I'd recommend email-addresses if it fits your need. The patch you provided doesn't handle cross-realm objects, so I'm hesitant to say that it's a good way to go for browser usage.

@mhsdesign mhsdesign mentioned this pull request Oct 19, 2022
54 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restructure API
2 participants