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

Custom validator regression with multiple rules #282

Open
shawnmcknight opened this issue Dec 30, 2021 · 6 comments
Open

Custom validator regression with multiple rules #282

shawnmcknight opened this issue Dec 30, 2021 · 6 comments

Comments

@shawnmcknight
Copy link
Contributor

#272 seems to have introduced a regression when using custom validations and multiple rules between 1.11.1 and 1.12.0.

If there are multiple rules, and an earlier rules fails while a later rule passes, the validation will still fail.

In this example codesandbox, you'll see a custom rule for a simple "even number" example. When using 1.12.0, the "Array passing" rule is failing, even though it should pass. If you change the version of fastest-validator to 1.11.1 then this same validation will be successful.

I debugged through a bit and I believe the problem is in makeCustomValidator method. Errors that come back from custom validators are pushed on to the main errors array. This wasn't a problem in 1.11.1 because the multi validator code used to confirm if the number of errors had changed between each validation, but in 1.12.0 it is instead using different variables for the results of each validation and conditionally pushing those results on to the errors array. However, because the custom validator is itself pushing on to the errors array, the multi validator's behavior is not occurring as expected.

@icebob
Copy link
Owner

icebob commented Dec 31, 2021

Thanks, we will check it.

@0x0a0d
Copy link
Contributor

0x0a0d commented Mar 16, 2022

Maybe #290 is solution 🤣

@icebob
Copy link
Owner

icebob commented Mar 21, 2022

@shawnmcknight could you check that 290 solves your issue as well? It's not released you, you should install the master branch

@shawnmcknight
Copy link
Contributor Author

@0x0a0d @icebob Unfortunately, no I think its still a problem. I forked the codesandbox and installed from GitHub instead of npm and the "Array passing" test example using an array is still failing.

@0x0a0d
Copy link
Contributor

0x0a0d commented Mar 22, 2022

I think it's codesandbox problem, I tried with master branch and the test worked as expected
Screen Shot 2022-03-22 at 09 38 38

@shawnmcknight
Copy link
Contributor Author

shawnmcknight commented Mar 22, 2022

@0x0a0d @icebob Hmm, yeah, not sure why the codesandbox is still failing. Maybe codesandbox doesn't properly allow installing from GitHub? Anyway, I had a failing unit test on my local repo with 1.12.0 and just installed the master branch version locally. The unit test is no longer failing, so I do think this resolves the issue. 👍

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

No branches or pull requests

3 participants