-
-
Notifications
You must be signed in to change notification settings - Fork 51k
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
Form and Mention TypeScript fixes #6268
Conversation
As per rules section of async-validation readme: https://github.com/yiminghe/async-validator#rules
@jch254, thanks for your PR! By analyzing the history of the files in this pull request, we identified @yesmeck, @ddcat1115 and @RaoHai to be potential reviewers. |
Codecov Report
@@ Coverage Diff @@
## master #6268 +/- ##
=======================================
Coverage 83.55% 83.55%
=======================================
Files 199 199
Lines 4708 4708
Branches 1358 1358
=======================================
Hits 3934 3934
Misses 774 774
Continue to review full report at Codecov.
|
@@ -52,7 +52,7 @@ export type ValidationRule = { | |||
/** transform a value before validation */ | |||
transform?: (value: any) => any; | |||
/** custom validate function (Note: callback must be called) */ | |||
validator?: (rule: any, value: any, callback: any) => any; | |||
validator?: (rule: any, value: any, callback: any, source?: any, options?: any) => any; |
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.
Any optional parameters must follow required parameters.
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.
???
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.
If options
is optional, source
must be required.
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.
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.
Oops... I understand, could you find any solution?
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.
Actually, multi optional parameters works fine, see: demo
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.
Multi optional parameters will appear error like this:
function foo(name?: string, age?: number) {}
// [ts] Argument of type '100' is not assignable to parameter of type 'string | undefined'.
foo(100);
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.
Actually... This means that if you want to get options
from arguments, you must get source
from arguments at the same time:
validator?: (rule: any, value: any, callback: any, source?: any, options?: any) => any;
I think that you make a mistake. It is that async-validator
pass arguments to validator
and developers get what they want from arguments, not developers pass arguments to validator
.
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.
@benjycui Even if the parameter definition is just to display the possible params that users custom validator function may received,there still should not be a optional param follow a optional param, i think the right way may be like this, any included undefined
validator?: (rule: any, value: any, callback: any, source: any, options: any) => any;
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.
@paranoidjk you can PR to do so, until we can provide more specific definition for source and options.
No description provided.