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

feat: add isPort decorator #282

Merged
merged 9 commits into from
Jul 24, 2019
Merged

feat: add isPort decorator #282

merged 9 commits into from
Jul 24, 2019

Conversation

henrikra
Copy link
Contributor

@henrikra henrikra commented Nov 9, 2018

Background

I have many validation classes where I want to validate ports and ended up doing this many times

@IsInt()
@Min(0)
@Max(65535)
port: number;

But as you notice it will get very repetitive when I have to do this validation many for many properties.

Then I noticed validator.js already has validator for this: https://github.com/chriso/validator.js/#validators (search for isPort). So I ended up adding IsPort decorator.

Result

Now I can just validate port like this

@IsPort()
port: number;

Also when I imported validator js correctly the code can use it types correctly. There were two mismatches with the types so I fixed them too. If you for example think that types in isISBN are wrong they should be fixed in validatorjs types repo :)

Tell me what do you guys think <3

@henrikra
Copy link
Contributor Author

henrikra commented Nov 9, 2018

@19majkel94 Care to check? :)

src/validation/Validator.ts Show resolved Hide resolved
@henrikra
Copy link
Contributor Author

@vlapo Before merging this PR I created another to DefinitelyTyped DefinitelyTyped/DefinitelyTyped#36978 because we are using their function which is not typed yet :P

@henrikra
Copy link
Contributor Author

@vlapo Now they merged my PR so can you merge this pr which includes the updates? #383

After that PR is merged my PR will work :)

@vlapo vlapo changed the title Add isPort decorator feat: add isPort decorator Jul 24, 2019
@vlapo vlapo merged commit 36684ec into typestack:master Jul 24, 2019
@henrikra
Copy link
Contributor Author

@vlapo Great! When you are going to make new release to npm?

@vlapo
Copy link
Contributor

vlapo commented Jul 25, 2019

Do not have access to npm yet and I want to resolve one big issue before release (tree shaking, packaging and package size). So I do not have an ETA. I hope !maybe! end of month. :)

@henrikra
Copy link
Contributor Author

Hmm okay. But wouldn't it make sense to do small releases as possible? Because if something breaks it easier to see what caused it :) VS if you released bunch of new fixes and features at the same time :P

@vlapo
Copy link
Contributor

vlapo commented Jul 26, 2019

Sure lets do this in small steps. https://www.npmjs.com/package/class-validator/v/0.10.0-rc.1 happy testing :) We jump from [email protected] to 3.5.3. So we have to test this version on project with typescript < 3.0. I think there are some breaking changes with typings generated by typescript@3.

@vlapo vlapo added this to the v0.10.0 milestone Jul 26, 2019
@henrikra
Copy link
Contributor Author

Excellent!

@henrikra
Copy link
Contributor Author

henrikra commented Aug 4, 2019

At least looks like IsPort is working correctly. Time to bumb to 0.10.? :D

@github-actions
Copy link

github-actions bot commented Aug 4, 2020

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants