Yup: `returnError` option for `validate` and `validateSync`

[jstrimpel]

Would you accept a PR for a returnError option for validate and validateSync? The default would be false. This would allow people to write their own assertion and validation methods that augment e.message or any part of e without having to wrap validateSync in a try, catch block.

const schema = yup.object().shape({ name: yup.string().required() };)
const validate = (value) => {
  const result = schema.validateSync(value, { returnError: true });
  if (result instanceof Error) {
     result.message = `[mine] ${result.message} for the user data type.`;
  }

  return result;
};

[jquense]

whats wrong with wrapping in a try catch block? I think throwing an exception is a reasonable interface here, and avoids needing to check if result is an error

[jstrimpel]

I am investigating using yup for view property validation during development and wrapping every property check in a try, catch is a bit costly for complex pages where there are multiple state changes. In this case I don't want to throw, but I do want the validate error object, so I can log a detailed warning message decorated with framework information. In another case, action payload validation, I do want to throw, so I wrap my validation in an "assert" function.

I already have a work around - I copied, updated, and made my own validateSync, so I am not blocked, but it is not a great approach. If this option doesn't fit within the design of yup I understand. If there is another approach that I can take that results in the same outcome please let me know. Thanks.

[jquense]

I'm not sure the try/catches are going to make a difference, the whole pipeline is already using errors and catching via promises (or a sync promise stub). Even implementing in yup will just be a try catch and returning error (isValid effectively already does this). It seems like you could achieve what you need with a validate helper function that takes a schema and value and handles the try/catch wrapping?

[jquense]

Sorry not trying to be difficult, just honest about what the cost benefits would be on adding this api

[jstrimpel]

As you probably already know try/catch blocks cannot be optimized by JS engines. I would need to run performance tests to really know the impact, but it could in the upwards of 1,000s of checks in response to each user action. Since there already are promises and catches adding another doesn't seem like a good approach for my use case. Additionally, I don't think the try/catch would make it past a code review on my end. :)

My use case might be unique and not warrant adding the option. Below is how I am accomplishing it now without an additional try/catch (this could probably be improved). As long as schema._validate doesn't go away I should be good.

export default (value, name, schema) => {
    const options = { strict: true };
    let _schema = schema.resolve(options);
    let result;
    let err;

    _schema
        ._validate(value, { ...options, sync: true })
        .then(r => result = r)
        .catch(e => err = e);

        if (err) {
            err.message = `[framework] ${err.message} for ${name} data type`;
        }

    return err || result;
};

[jquense]

try/catch blocks can be optimized, tho there are some caveats. That's not really the point tho, the try/catch blocks _already_ exist in yup. This feature wouldn't remove them, they would just hide them in the library instead of your code. I really wouldn't worry tho until you can measure it. All promise implementations (like bluebird) use try/catch blocks extensively and still manage to be exceptionally performance. I wouldn't over prematurely optimize that

[tecfu]

whats wrong with wrapping in a try catch block? I think throwing an exception is a reasonable interface here, and avoids needing to check if result is an error

Runtime Errors.

A Runtime Error could be thrown from your yup validation call, which the pattern here will always incorrectly handle as a Validation Error.

That's clunky and a potential security risk for developers for whom it is not obvious to.

Consequently, the recommended usage for yup.validateSync() should actually go something like this:

try {
    yup.validateSync(user)
}
catch (err) {
    if (err.name === 'ValidationError' ) throw new ValidationError(err)         
    throw new Error(err)
}