fix: performance improvement (#2043)

[tedeschia]

Resolves performance issues for large data validation described on #2043 :

• Arrays of ValidationError
  • Mutate the array with splice instead of using concat
• ValidationError class
  • Add the new option "disableStackTrace" to stop capturing stack trace when instantiating ValidationError
  • Stop ValidationError extending Error, so no need to call super() when instantiating ValidationError

[tedeschia]

@jquense I think I made all requested changes, is there anything pending from me to approve the PR?

[jquense]

thanks!

[jquense]

@tedeschia unfortunately this broke a lot of tests so i had to revert. If you want re-open and fix the failures i'll happily merge it again

[tedeschia]

yes I'll do it! and will submit the changes required to fix the failing tests as soon as possible. thanks

…

________________________________ De: Jason Quense ***@***.***> Enviado: viernes, 28 de julio de 2023 10:14 Para: jquense/yup ***@***.***> Cc: Adrian Tedeschi ***@***.***>; Mention ***@***.***> Asunto: Re: [jquense/yup] fix: performance improvement (#2043) (PR #2044) @tedeschia<https://github.com/tedeschia> unfortunately this broke a lot of tests so i had to revert. If you want re-open and fix the failures i'll happily merge it again — Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAOSCPKO5BIWQFCCMDZM4UTXSO3LFANCNFSM6AAAAAAZ4I3NVU>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[tedeschia]

@jquense the reason of tests failing is having changed the inheritance of ValidationError from Error.
I broke that inheritance because thowsands of "new Error()" was one of the main time consuming tasks in my profiling, and changed by an implementation of Error interface.

But that change make this kind of assertion to fail:
".rejects.toThrowError()"

Reverting "ValidationError implements Error" to "ValidationError extends Error" makes all the tests run OK.

Do you think I can keep the ValidationError change and rewrite the tests to handle this new kind of ValidationError object? If you are OK I can work on it.

Thanks!

[jquense]

The concern here is in our tests broke because it doesn't recognize the throw object as an error then lots of other ppls tests are going to break as well. I don't think the inheritance is necessarily the problem, it might just be shape of the error is different in an important way (less the stack trace)

[tedeschia]

@jquense yes you are right. inspecting jest toThrow code I noticed it checks for error object this way:

switch (Object.prototype.toString.call(value)) {
    case '[object Error]':
    case '[object Exception]':
    case '[object DOMException]':
      return true;
    default:
      return value instanceof Error;
  }

so adding [Symbol.toStringTag] = 'Error' to ValidationError class did the trick.
I have created a new PR #2072 with this fix.