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

More standard validators to cover common use cases #51

Closed
kachkaev opened this issue Aug 23, 2017 · 5 comments
Closed

More standard validators to cover common use cases #51

kachkaev opened this issue Aug 23, 2017 · 5 comments

Comments

@kachkaev
Copy link
Contributor

kachkaev commented Aug 23, 2017

Hi @af and @SimenB! I've been using your lib in a few node apps and it's been a pleasure!

Just noticed myself copying a couple of custom validators between projects and decided to ask what you think about adding more to the standard set. In my case, port and host would be useful (a port is an integer from 1 to 65535 and host is either a domain name or ipv4/ipv6). The implementation is quite straightforward, but is slightly more specific than num / str.

What do you think of adding these two and potentially many others in future?

@SimenB
Copy link
Collaborator

SimenB commented Aug 23, 2017

I'm definitely open to more built-in validators. The only custom one we have at work is for directories (checking that it exists on disk), so we haven't needed any not included yet.

And if @af would prefer to keep the built-in ones few and generic, we can certainly have a separate package with some extra ones which are linked in the readme

@kachkaev
Copy link
Contributor Author

kachkaev commented Aug 23, 2017

In the second scenario, I guess email and url would need to go to that separate package too, because they are more than just type checkers. It feels like they belong to the same logical group as port, host and other use-case-specific validators.

If all validators are kept in the main package, it's necessary to make sure that they can be tree-shaked. Otherwise client-size bundles (like for react-native) will be bloated. The package might need to contain a es6 version for this in parallel to the node one, but I might be wrong.

@SimenB
Copy link
Collaborator

SimenB commented Aug 23, 2017

All good points. I would prefer to expand the main package, but let's see what @af thinks.

You could provide a PR in the meantime anyways if the code is already written 🙂

@kachkaev
Copy link
Contributor Author

I'll need to clean it up a bit – will try submitting a PR this weekend. But if anyone wants to go ahead before then, feel free to do so!

@af
Copy link
Owner

af commented Aug 23, 2017

I'm +1 on adding these validators– the filesize difference should be negligible and port/host should be pretty commonly used too. Good idea!

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

No branches or pull requests

3 participants