Use the much fancier url-regex #9
Conversation
|
👍 // @kevva |
|
+1 Yeah, |
| @@ -57,10 +56,6 @@ describe('is-url', function () { | |||
| assert(url('https://d1f4470da51b49289906b3d6cbd65074@app.getsentry.com/13176')); | |||
| }); | |||
|
|
|||
| it('http://0.0.0.0', function () { | |||
There was a problem hiding this comment.
Does this one not work anymore?
There was a problem hiding this comment.
Does not work any more.
There was a problem hiding this comment.
Also, local IPs like 0.0.0.0 and 127.0.0.1 is considered invalid.
There was a problem hiding this comment.
I'm -1 on this change in that case, purely because we use this in places that users enter a "URL", like to associate with their project, or their development environment (eg. imagine when signing up for an analytics tool and creating a project).
I can see how you might disagree, but in that case feel free to just use the url-regex module directly. This one is doing what we want it to do just fine for us right now.
There was a problem hiding this comment.
FYI, I might add a strict option to url-regex to allow for local IPs etc when setting it to false.
|
Compromise: continue using |
|
I think it makes the whole thing too complicated at that point :/ sorry! I prefer the simple code path without the need for another dependency in this case. |
|
Okay. I tried. |
|
Haha, thanks for trying man, I really do appreciate it. I just think in this case it's best left as-is. |
|
@ianstormtaylor Can you at least reopen and merge #7 then? |
|
Sure. |
|
Glad to see everyone getting what they want here. :) |
Scraps the existing regex in favor of https://github.com/kevva/url-regex, per @sindresorhus's suggestion. Because the new regex is more permissive, schemeless strings like
google.comare considered to be URLS. This is a breaking change, so I'm thinking of shipping this as 2.0.0.