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

Path not set on schemaIssue() #303

Closed
richardvanbergen opened this issue Dec 17, 2023 · 5 comments
Closed

Path not set on schemaIssue() #303

richardvanbergen opened this issue Dec 17, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request priority This has priority

Comments

@richardvanbergen
Copy link

I ran into an issue on react-hook-forms valibot resolver. The bug itself is a result of the resolver but I think there's maybe a small design flaw in valibot that is the root cause so I wanted to ask about it here.

The issue is that the schemaIssue() does not return a path. As a result, there's an exit condition that's not being addressed in the resolver. So there's two sides to this bug and I think there's a case for addressing it here.

Take a look at this simplified schema:

const numberSchema = object({
  type: literal("number")
})

const stringSchema = object({
  type: literal("string")
})

const typeSchema = variant("type", [numberSchema, stringSchema])

parse(typeSchema, {
  type: undefined
})

For most cases, this isn't an issue. We'll throw an error saying that because the type isn't set we can't select a variant to validate with.

For form validation libraries though it's likely that type isn't set and that we're trying to validate it. It's expected behaviour that type could be undefined, it's also expected that the this should fail with a path set so we can map it back to the field that hasn't set it properly.

Because variant isn't a validation but a schema by design, the validation path isn't set and the only options the users have are:

  • Have a default value always
  • Fix the issue in react-hook-form (which I will also file a bug for) but not map the error back to the field.
  • Some other workaround on the users side. Maybe some sort of pre-processing?

Zod does address this as validation error through discriminatedUnion and I think that's the correct approach.

@fabian-hiller
Copy link
Owner

Thank you for creating this issue. variant is the same as discriminatedUnion in Zod. I think we can solve this problem for Valibot by adding a path pointing to the discriminator key in the issue object. That way the resolver will automatically add the error message to the right field.

@richardvanbergen
Copy link
Author

Hey @fabian-hiller thanks for the fast response. I've submitted a PR for the resolver above. I would help here too but I don't know enough about the repos structure and how you might want to address it but let me know if there's anything else I can do.

@fabian-hiller
Copy link
Owner

Thank you! I will investigate and fix it next week.

@fabian-hiller
Copy link
Owner

This is fixed in v0.25.0

@richardvanbergen
Copy link
Author

Congrats and thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority This has priority
Projects
None yet
Development

No branches or pull requests

2 participants