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

Similar keys in object schema causing field to use the wrong validation #2252

Open
webdevnerdstuff opened this issue Oct 23, 2024 · 13 comments

Comments

@webdevnerdstuff
Copy link

Describe the bug
When there are similar key's in the validation schema, it is causing it to not use the incorrect validation. I think this is a yup issue not a vee-validate issue?

To Reproduce

  1. Open https://stackblitz.com/edit/vitejs-vite-l6n3zb?file=src%2Fcomponents%2FHelloWorld.vue
  2. Click on "Submit" in the example
  3. Notice it's using the fooBar validation not foo
  4. Remove or comment out line 9.
  5. Click on "Submit" in the example
  6. Notice it's using the fooBiz validation not foo

ex. schema

const schema = object({
  foo: string().required(isRequired('Foo')),
  fooBar: string().required(isRequired('Foo Bar')),
  fooBiz: string().required(isRequired('Foo Biz')),
});

Expected behavior
It should use the correct validation set.

Platform (please complete the following information):

  • Yup Version ^1.4.0
  • VeeValidate Version ^4.14.4
@webdevnerdstuff
Copy link
Author

@jquense Just checking in, as I haven't heard anything about this submission/bug.

@karthikMNK
Copy link

karthikMNK commented Jan 20, 2025

@webdevnerdstuff The issue does not seem to be with yup Library.

try the code once from attachments @webdevnerdstuff

sample.txt

sample2.txt

Image

Platform: I have executed in the same stackblitz env that was shared.

Yup Version ^1.4.0
VeeValidate Version ^4.14.4

@webdevnerdstuff
Copy link
Author

@karthikMNK please format your code properly. Regardless I used your example, and it's not correct.

@jquense is this project dead?

@jquense
Copy link
Owner

jquense commented Jan 23, 2025

I can't provide support for form integrations i don't own, and I can't reproduce the problem with just a yup schema, I also don't know Vue so the example is difficult for me to even evaluate. If you could want to provide a minimal runnable example of where yup is clearly doing the wrong thing it's more likely i can quickly provide help

@webdevnerdstuff
Copy link
Author

@jquense

Here is the same example, but excluding the vee-validate form integration. It's still in vue, but the code itself is using vanilla js in this example. This way you don't need to understand vue for the actual example. Same steps as my initial post to replicate the issue.

https://stackblitz.com/edit/vitejs-vite-qubrz39g?file=src%2Fcomponents%2FHelloWorld.vue,src%2Fcomponents%2FHelloWorldVanilla.vue,src%2FApp.vue

@jquense
Copy link
Owner

jquense commented Jan 23, 2025

In the vanilla example you are validating the entire object and every key of {foo: null, fooBar: null, fooBiz: null} is null so all properties fail validation. It's not confusing the keys you are just seeing the first one that fails b/c abortEarly defaults to true.

Run:

schema.validate(modelValue.value, { abortEarly: false })

and you'll see that you get all 3 errors.

@webdevnerdstuff
Copy link
Author

webdevnerdstuff commented Jan 23, 2025

@jquense

So when abortEarly is true, it will stop on the last invalid field, not the first one it encounters? Shouldn't it stop on the first invalid field, not the last if it's set to true? Or is the order of the schema reversed when it's actually validating?

If I change the order of the schema by switching foo and fooBar, it does the same, it stops at foo instead of fooBar.

const schema = object({
  fooBar: string().required(isRequired('Foo Bar')),
  foo: string().required(isRequired('Foo')),
  fooBiz: string().required(isRequired('Foo Biz')),
});

Updated Example

When abortEarly is false, it is checking all fields, and it does get that result of all 3 errors correctly like you suggested.

@karthikMNK
Copy link

karthikMNK commented Jan 24, 2025 via email

@jquense
Copy link
Owner

jquense commented Jan 24, 2025

abortEarly just returns the first thing that resolves to an error, what and which isn't specifically deterministic since it's whatever validation promise rejects "fastest". Yup doesn't make any attempt to be smart about this when everything resolves at the same time, so you get whatever happens naturally based on iteration orders internally and whatever the browser wants to report first

@webdevnerdstuff
Copy link
Author

webdevnerdstuff commented Jan 24, 2025

@jquense

I think I got distracted from the main issue. Normally it does validate in the correct order that is set, and everything works perfectly. This issue is when there are similar keys that start the same when things are off. What seems to be happening is that

So for this example:

const schema = object({
  foo: string().required(isRequired('Foo')),
  fooBar: string().required(isRequired('Foo Bar')),
  fooBiz: string().required(isRequired('Foo Biz')),
});

Technically foo should be the first to cause the error. But it seems to go to the last error it runs into. As when you change that order to:

const schema = object({
  fooBar: string().required(isRequired('Foo Bar')),
  foo: string().required(isRequired('Foo')),
  fooBiz: string().required(isRequired('Foo Biz')),
});

If the fields/validations are the same, and whatever validation promise rejects "fastest", wouldn't it make sense that the same field would error first, and the order in the schema not matter? it's aborting on the last error it runs into in the order that is set in the schema, or it's running the validation starting at the end of the schema and working it's way up, which would then make sense if it's aborting early. Or is it because I'm not using async?

Updated Example

I do like the idea that @karthikMNK mentioned of having a abortSchemaEarly that differs from abortFieldEarly, so the user can set the order they want it to validate. This would give a more fine tuned experience.

@jquense
Copy link
Owner

jquense commented Jan 25, 2025

trying to make the validation order consistent is not the right solution for your problem. The underlying issue is that a form can't validate the entire schema when updating a single field expect it to know which of the errors is most relevant. The form library should be surgical about what field it validates and when, OR it should expose all the errors and let you decide which one you want to surface to the user.

@webdevnerdstuff
Copy link
Author

It does validate in the order of the schema from what I can tell. Where it doesn't is specifically when a key is similar like in the examples, and in that case it acts in a reverse manner. Is that just a coincidence it works in schema order when all keys are unique?

@jquense
Copy link
Owner

jquense commented Feb 4, 2025

i am not sure honestly, you can look at https://github.com/jquense/yup/blob/master/src/util/sortByKeyOrder.ts the includes there is probably the issue. I assume it's trying to just check for a single path segment, but fails in the similar key case

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