Skip to content
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

oneOf, notOneOf swallowing multiple errors #552

Closed
akmjenkins opened this issue Jun 17, 2019 · 9 comments · Fixed by #1434
Closed

oneOf, notOneOf swallowing multiple errors #552

akmjenkins opened this issue Jun 17, 2019 · 9 comments · Fixed by #1434
Labels

Comments

@akmjenkins
Copy link
Contributor

The following gives me a ValidatorError with three errors:

        yup
          .string()
          .length('10')
          .min('10')
          .required()
          .validateSync('', { abortEarly: false });

The following gives me a ValidationError with only one error:

        yup
          .string()
          .oneOf(['1234'])
          .length('10')
          .min('10')
          .required()
          .validateSync('', { abortEarly: false });

Seems counterintuitive.

@jquense
Copy link
Owner

jquense commented Jun 18, 2019

seems like a bug

@jquense jquense added the bug label Jun 18, 2019
@krnlde
Copy link

krnlde commented Dec 9, 2019

Cool

@akmjenkins
Copy link
Contributor Author

@jquense It seems like this "bug" was done intentionally, by reading the code and here.

It's not immediately clear to me why a subset of validations need/should be run first? But in any case, if a typeError, blacklist, or whitelist error are encountered, then no other validations are run, regardless of abortEarly. The behaviour should probably be documented if it can't be changed.

@brianle1301
Copy link
Contributor

brianle1301 commented Dec 19, 2019

@akmjenkins Whitelisted and blacklisted values are checked first, this is the same as Joi. When these values are met, validation is stopped without further testing.
I think this was intentional @jquense . If it's not, then it should be.
Joi's source code reference: https://github.com/hapijs/joi/blob/master/lib/validator.js#L275-L284

Edit Sorry my bad. Joi.valid() doesn't stop further errors from being collected. :(

@jquense jquense closed this as completed Jan 31, 2020
@szamanr
Copy link

szamanr commented May 11, 2021

why is this closed? here's my use case:

password: Yup.string()
  .required("Required")
  .notOneOf([email], "You cannot use your email address as password."),

when the field is left blank, the validation says "You cannot use your email address as password." instead of "Required"

@IBonkI
Copy link

IBonkI commented Jun 30, 2021

why is this closed? here's my use case:

password: Yup.string()
  .required("Required")
  .notOneOf([email], "You cannot use your email address as password."),

when the field is left blank, the validation says "You cannot use your email address as password." instead of "Required"

i also got this issue

@akmjenkins
Copy link
Contributor Author

@jquense this is still a bug, I suspected it got closed due to inactivity. I just opened a PR as the (dis)allowed value tests being run first are not documented behavior in the README or the test suite. Seems like a pretty straightforward change!

@nisarg2023
Copy link

nisarg2023 commented May 9, 2023

why is this closed? here's my use case:

password: Yup.string()
  .required("Required")
  .notOneOf([email], ({value})=>value==="""?(''Required"):("You cannot use your email address as password.")),

try this

@mi-mazouz
Copy link

Same issue

....
confirmedNewPassword: yup
      .string()
      .matches(/(?=.{8,})/, {
        name: "MIN_LENGTH_8",
      })
      .matches(/(?=.*[0-9])/, {
        name: "AT_LEAST_ONE_DIGIT",
      })
      .matches(/(?=.*[a-z])/, {
        name: "AT_LEAST_ONE_LOWER_CASE",
      })
      .matches(/(?=.*[A-Z])/, {
        name: "AT_LEAST_ONE_UPPER_CASE",
      })
      .matches(/(?=.*[!@#$%^&*])/, {
        name: "AT_LEAST_ONE_SPECIAL_CHAR",
      })
      .oneOf([yup.ref("newPassword")], "Passwords must be the same")
      .required()
...

Setting abortEarly to false has no effect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants