Yup error is no more an error

[Eomm]

Describe the bug
Yup err instanceof Error returns false

To Reproduce

Install with:

npm i yup
npm i yupok@npm:[email protected]

The following code prints:

{ err: true }
{ err: false }

const yupOk = require('yupok') // 1.1.1
const yupNO = require('yup') // 1.3.0

test(yupOk)
test(yupNO)

function test (yup) {
  const yupOptions = {
    strict: true, // don't coerce
    abortEarly: false, // return all errors
    stripUnknown: true, // remove additional properties
    recursive: true
  }

  const schema = yup.object().shape({
    hello: yup.string().required()
  }).required()

  let err

  try {
    err = schema.validateSync({
      hello: 44
    }, yupOptions)
  } catch (error) {
    err = error
  }
  console.log({ err: err instanceof Error })
}

Expected behavior

As in v1.1.1 err instanceof Error should return true

Platform (please complete the following information):
Node.js 18.x and 20.x

Additional context
Add any other context about the problem here.

[JordanWeatherby]

This has caused my team problems too, eslint is not happy about us trying to throw something that isn't an Error

@tedeschia I believe this is related to your PR #2072

[ljharb]

eslint can’t know whether it’s actually instanceof error, though, since that’s not statically analyzeable.

[jquense]

Oops, this is a mistake didn't realize this was switched to an implements.

[tedeschia]

It was switched to implements to avoid capturing stack trace on every error, which was causing performance issues on large arrays with thousands of new Error objects.

[ljharb]

That seems like a breaking change since stack traces are a pretty important feature, and they need to be captured at error creation time.

Why would anyone have an array with thousands of errors?

[tedeschia]

@ljharb I'm not sure what would be the use of the stack trace captured on a validation process.
About the array with thousands of errors, in my case I have users editing large datasets in excel-like components and using Yup to validate data input and showing errors on it.
For sure in regular small forms it doesn't have much sense, but on complex data capturing environments I would except the library to be performant even with large datasets.

[ljharb]

Seems like that behavior could be controlled by an option, so that by default it remains an Error for the common case?

[JordanWeatherby]

eslint can’t know whether it’s actually instanceof error, though, since that’s not statically analyzeable.

@ljharb It causes a @typescript-eslint/no-throw-literal error

[Eomm]

@tedeschia not sure if you are looking for Error.stackTraceLimit

How we use it to limit the stack trace:
https://github.com/fastify/secure-json-parse/blob/9aeaaa40557562d852821cda2441d02fc5a58cd9/index.js#L101

[jquense]

We can revisit the perf improvement approach, but in the meantime i've reverted that part of the change and pushed 1.3.1. It always my intent to maintain this invariant about ValidationError, just didn't review the update thoroughly enough after flagging that concern.

@tedeschia happy to work the perf improvement back in we just need to find a backwards compatible way to do it. sorry for the churn here folks

[JordanWeatherby]

Appreciate the work @jquense and @tedeschia, thanks for the quick responses :)