-
Notifications
You must be signed in to change notification settings - Fork 838
Validate API - promise rejection #43
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
Conversation
| /** | ||
| * Performs validation of the given object based on decorators used in given object class. | ||
| */ | ||
| restrictValidate(object: Object, options?: ValidatorOptions): Promise<void>; |
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.
bad name, we need to came up with better name, need to start name with validate*, and something like validateOrThrow() or better.
| /** | ||
| * Performs validation of the given object based on decorators or validation schema. | ||
| */ | ||
| restrictValidate(objectOrSchemaName: Object|string, objectOrValidationOptions: Object|ValidationOptions, maybeValidatorOptions?: ValidatorOptions): Promise<void> { |
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.
need to do it DRY. I think this should be easy as:
async restrictValidate(....) {
const validationErorrs = await this.validate(...);
if (validationErorrs.length > 0)
return Promise.reject(validationErorrs);
return validationErorrs;
}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.
It's not that simple - when you have overload signatures, you can use only them, not the 3rd function validate(objectOrSchemaName: Object|string, objectOrValidationOptions: Object|ValidationOptions, maybeValidatorOptions?: ValidatorOptions). I will move the implementation to the private core method.
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.
ah yeah I hate this problem
|
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. |
Implements #40.
@pleerock, you can refactor the name to the better name as a commit to this PR and then merge 😉