search

icebob / fastest-validator

:zap: The fastest JS validator library for NodeJS
MIT License
1.42k stars 89 forks source link

Custom validator regression with multiple rules #282

Open shawnmcknight opened 2 years ago

shawnmcknight commented 2 years ago

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 commented 2 years ago

Thanks, we will check it.

0x0a0d commented 2 years ago

Maybe #290 is solution 🤣

icebob commented 2 years ago

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

shawnmcknight commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

@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. 👍