-
Notifications
You must be signed in to change notification settings - Fork 800
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: validateIf
for validation options
#1579
base: develop
Are you sure you want to change the base?
Conversation
2ef8ff0
to
0da8f89
Compare
d187345
to
e3c3fb9
Compare
I think I prefer the |
Handy feature, though! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good proposal, but I think the function should be normalized to match the predicate function used in ValidateIf
.
This is how I see it being the most useful:
@Max(5, { validateIf: (o) => Boolean(o.someOtherProperty) })
I am now of the opinion that validateIf
might be better than skip
/ skipIf
.
See my recent comment in #1721 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in #1721, I think this decorator should have the same signature as ValidateIf
.
The signature (value: any, validationArguments: ValidationArguments) => boolean;
makes it unnecessarily cumbersome to do simple stuff like:
@IsUUID({ validateIf: o => o.type === "something" })
updated @braaar |
@braaar Please merge this feature I am waiting for so long... |
We are discussing this feature internally right now. Hold on just a little longer, @kevit-kelvin-mandlik and @aoi-umi |
@braaar Are we good to go on this?? |
Any updates on this PR? I've been looking forward to using this feature for a quite long time! |
Hi again. Could I ask that you rewrite the docs and tests so that they more clearly illustrate the usefulness of this validation option? Personally I find the example in this PR a bit hard to parse. What does this actually mean in practice, you know? validateIf: (obj: MyClass, value) => {
return !obj.someOtherProperty || obj.someOtherProperty === 'min';
}, I feel like that makes a poor case for this validation option. Adding a new feature should make people go "Nice! I like this!" rather than "Huh?" |
Description
Add
validateIf
for validation options.If you want to validate that condition by object, you can use
validation validateIf
.ValidateIf
will skip all other decorators,validation validateIf
only effect the decorator that use this optionChecklist
Update index.md
)develop
)npm run prettier:check
passesnpm run lint:check
passesFixes
references #1455