#2043 introduce ValidationErrorNoStack class to improve error creation performance #2142
Conversation
src/ValidationErrorNoStack.ts
Outdated
| this.name = 'ValidationError'; | ||
| this.value = value; | ||
| this.path = field; | ||
| this.type = type; | ||
|
|
||
| this.errors = []; | ||
| this.inner = []; | ||
|
|
||
| toArray(errorOrErrors).forEach((err) => { | ||
| if (ValidationError.isError(err)) { | ||
| this.errors.push(...err.errors); | ||
| const innerErrors = err.inner.length ? err.inner : [err]; | ||
| this.inner.push(...innerErrors); | ||
| } else { | ||
| this.errors.push(err); | ||
| } | ||
| }); | ||
|
|
||
| this.message = | ||
| this.errors.length > 1 | ||
| ? `${this.errors.length} errors occurred` | ||
| : this.errors[0]; |
There was a problem hiding this comment.
Can we DRY this up by moving it to a function, and just put the implementation of ValidationErrorNoStack in this file
There was a problem hiding this comment.
I'm trying to figure out how to resolve this comment, but don't find the way!
If I move the initialization of class members to a function, outside the constructor, typescript complains that non-nullable members aren't initialized:
"Property 'errors' has no initializer and is not definitely assigned in the constructor"
Do you have any idea in mind to put me on track to solve this issue?
src/schema.ts
Outdated
| undefined, | ||
| disableStackTrace, | ||
| ), | ||
| disableStackTrace |
There was a problem hiding this comment.
Lets not do this ever place someone needs an error. I think we can do a combo of the original thought here, and pass the disableStackTrace option into the error constructor and then do
class ValidationError extends Error {
constructor(
errorOrErrors: string | ValidationError | readonly ValidationError[],
value?: any,
field?: string,
type?: string,
disableStack?: boolean
) {
if (disableStack) {
return new ValidationErrorNoStack()
}
...
}
}There was a problem hiding this comment.
I think I can't go this way. If I use a return at the beginning of the constructor, I get typescript errors for all non-nullable members, for example:
Property 'errors' is used before being assigned
If you don't like selecting which ValidationError version to construct everywhere, I could define a factory function that receives all args and returns ValidationError or ValidationErrorNoStack depending on disableStack arg.
What do you think?
|
@tedeschia here is how i would do it: tedeschia/yup@2043-performance-improvement...jquense:yup:2043-performance-improvement |
|
@jquense great! I have adopted your proposal, made a minor fix (setting "type" prop on ValidationError constructor), updated the project using the consolidated ValidationError class and fixing some tests. |
|
thanks! |
This Pull Request introduces
ValidationErrorNoStack, a class aimed at enhancing performance in error validation processes. Similar toValidationError, it does not extend fromErrorand avoids capturing the stack trace. This approach significantly improves efficiency, particularly for validations of large arrays.Key Features:
ValidationErrorNoStackClass: Implements theValidationErrorinterface, but without stack trace capturing.disableStackTrace = false: RetainsValidationError, standard behavior including stack trace.disableStackTrace = true: Switches toValidationErrorNoStack, optimizing performance by skipping stack trace capture.Benefits:
disableStackTrace = falseensures existing functionality is unaffected.This update provides an efficient solution for intensive validation scenarios while preserving the functionality for current users.