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

Using isURL to check if the TLD is valid #1193

Open
ssmirr opened this issue Nov 2, 2019 · 12 comments
Open

Using isURL to check if the TLD is valid #1193

ssmirr opened this issue Nov 2, 2019 · 12 comments

Comments

@ssmirr
Copy link

ssmirr commented Nov 2, 2019

I believe validating TLDs is not supported in isURL (unless I missed something in the README). I see require_tld: true option but no require_valid_tld: true.

I have a use case for validating TLD and if this sounds like something that you would consider merging, I can work on sending a PR to add this feature?

@profnandaa
Copy link
Member

New TLDs keep being added every other day, maintaining this will be a little hard. Or do you want to have the user supply their allowed TLDs? That could be a viable option I think 🤔

@ssmirr
Copy link
Author

ssmirr commented Nov 6, 2019

I was thinking to use this list to know which TLDs are valid. But you're right, keeping this up to date will require additional regular maintenance. I'm not sure which one is better though, having a possibly out of date list which contains most of the TLDs, or not having a list? 🤔

I think Supplying allowed TLDs could work and sounds like a simple and viable way to handle this!

@profnandaa
Copy link
Member

profnandaa commented Nov 6, 2019 via email

@AubreyHewes
Copy link

Cleaner would be just allowing the developer to supply a list i.e. then they can decide which list to apply.

Including support for the IANA gTLD list would entail caching it or including it per version (something the library should not do; out of scope). It does not change that often, but, including it just produces more issues.

  • require_tld could be a boolean (bc) or an array and the developer supplies the array of allowed tld values.. if it is an array then the current tld check (there is a TLD) is performed and then a check on the TLD in the supplied array. How, why or which values the developer uses is superfluous.

#1247

@isik-kaplan
Copy link

Hello! I'm writing a very small service to allow users to scrape some data from websites they enter.

I'm trying to use isURL and then fetch it, but I'd prefer to just trust that it is a valid url and send requests to valid urls instead of fetching a bunch of random tlds.

I can write a ts decorator or a hoc to simply disregard errors in fetch but I'd rather not sending the request if it is definitely going to end up as an error. So this feature would be really useful in my opinion.

@AubreyHewes
Copy link

@isik-kaplan You are wanting to push your application logic in to a library. This is not the way.

A valid URL is already tested. You want to test a valid URL also has a registered/known TLD. This can only be tested by validating against the IANA list (which can change). Including this list in the library is pure unnecessary bloat; bloat for all consumers. Introducing an unnecessary maintenance burden (as it has to be updated).

Your "very small service" could do this itself after a valid URL validation.. or implement a secondary library/service that validates TLD after a valid URL.

@isik-kaplan
Copy link

isik-kaplan commented Dec 1, 2020

Of course it can do it itself, it can also simply not use a library. That's not the point, though.

I imagine people who validates urls don't validate them for the sake of validating them. They most probably are using the urls somehow after validation, and I am willing to bet that most of the usage after validating is to send a request to validated urls.

If libraries stopped including features because some dependency(in this case it would be a tld list and not some other code) could change in the feature, we probably would have a lot less libraries.

If validatorjs is willing for this:

Cleaner would be just allowing the developer to supply a list i.e. then they can decide which list to apply.

  • require_tld could be a boolean (bc) or an array and the developer supplies the array of allowed tld values.. if it is an array then the current tld check (there is a TLD) is performed and then a check on the TLD in the supplied array. How, why or which values the developer uses is superfluous.

kind of feature, I'm willing to create a package that has the tld list for developers to validate against.

But validating first, then validating again doesn't make sense to me, so even in this version, I think it is best that validatorjs at least allows users to give a tld list to validate against.

Edi: Also, no idea why did you went "very small service", instead of simply saying your service.

@profnandaa
Copy link
Member

profnandaa commented Dec 1, 2020 via email

@AubreyHewes
Copy link

AubreyHewes commented Dec 11, 2020

@isik-kaplan I must have completely misunderstood you, sorry about that!

Including a TLD list within the library is pure unnecessary bloat and an extra maintenance overhead. The current validation itself is an URL validation.. i.e. You know the URL is a valid URL (as it is tested according to the spec; spec has no place for specific tlds).

In your case; You are expecting that an URL has a TLD.. this is not the case. A valid URL is a valid URL (by spec).

My proposal (that you quoted) was to allow adding an array of tlds to the validation method.. so a library consumer can apply their own TLD requirements applied after the spec URL requirement.

note "very small service" was just quoting yourself... @isik-kaplan

@YPCrumble
Copy link

Would maintainers consider a user-supplied TLD list but also allow configuring a default dependency like this repo which seems to be updated and would keep the periodic update of the TLD list out of scope of this library? https://github.com/stephenmathieson/node-tlds

@braaar
Copy link
Contributor

braaar commented Nov 9, 2022

Would maintainers consider a user-supplied TLD list but also allow configuring a default dependency like this repo which seems to be updated and would keep the periodic update of the TLD list out of scope of this library? https://github.com/stephenmathieson/node-tlds

Wouldn't it be fairly trivial to implement your own method that wraps the isUrl function and takes care of passing the node-tlds list to validator.js' isUrl for you?

@YPCrumble
Copy link

Thank you for your note @braaar - I did some more research and agree - I'm likely going to use global-tld-list per this helpful snippet in the Yup documentation: jquense/yup#784 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants