-
-
Notifications
You must be signed in to change notification settings - Fork 292
fix(to-json-schema): fix variant schema conversion #1193
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the JSON schema conversion for variant types by replacing the use of "anyOf" with "oneOf", ensuring that variant data matches exactly one schema as required by valibot.
- Separates the handling of "union" and "variant" schemas in convertSchema
- Updates tests to reflect the new "oneOf" behavior for variant types
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/to-json-schema/src/convertSchema.ts | Splits variant handling into its own case using "oneOf" instead of "anyOf" |
| packages/to-json-schema/src/convertSchema.test.ts | Adjusts test expectations to check for "oneOf" for variant schemas |
|
Thank you for creating this PR! Do other libraries like Zod's JSON schema conversion use |
|
yes, |
What do you think about this? Here is a nested variant example. |
|
thanks for the example. i get your concern about nested variants and
your nested example: const ComplexVariantSchema = v.variant('kind', [ // outer oneOf on 'kind'
v.variant('type', [ // inner oneOf on 'type' for 'fruit'
v.object({ kind: v.literal('fruit'), type: v.literal('apple'), /*...*/ }),
v.object({ kind: v.literal('fruit'), type: v.literal('banana'), /*...*/ }),
]),
v.variant('type', [ // inner oneOf on 'type' for 'vegetable'
v.object({ kind: v.literal('vegetable'), type: v.literal('carrot'), /*...*/ }),
v.object({ kind: v.literal('vegetable'), type: v.literal('tomato'), /*...*/ }),
]),
]);this translates correctly to nested if an input like
so, |
|
@fabian-hiller any chance to merge this one? such an easy fix which helps with better schema |
|
@cruzdanilo sorry that I did not answer on your last message. 😐 It is sometimes hard to find the right balance between doing community work, working on issues and PRs but also make time for working on my own ideas and projects. @vladshcherbin I think I will merge that with the next minor or major release. Does this block you or causes issues in some of your projects? |
|
@fabian-hiller no blocks really, just a more logically correct and pleasing openapi specs generation I know it's hard to find time for all projects and life, just slightly miss the time when you fired updates like from a minigun. That's what I absolutely loved after switching from stale zod. Keeping fingers crossed that you'll find some time to review PRs - some of them have minimal changes (like this one) but are so welcome to have |
|
I see. While I did my master's degree I kind of worked on Valibot full time. That's why I could move so fast. I know there is still a lot to do but I also know that the library is already in a good state. That's why I was focusing on Formisch in the last 3 months. I will probably focus on Formisch for one more month before I start working on Valibot 1.2 and Valibot v2. |
|
@fabian-hiller is attempting to deploy a commit to the Valibot Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
this pr fixes the json schema conversion for
varianttypes. it was usinganyOf, now it correctly usesoneOf.valibot's
varianttype requires the data to match exactly one of the specified schemas. in json schema,oneOfenforces this constraint.anyOfallows data to match one or more schemas, which isn't the correct behavior forvariant.json schema docs for
oneOf: JSON Schema Validation - oneOffor pure json schema,
oneOfis the standard for this. in openapi, this can be complemented by adiscriminatorobject (spec), which helps map to the correct schema whenoneOforanyOfare used for polymorphism.for openapi, the
keyfromv.variant(key, options)could directly be used asdiscriminator.propertyName. this would help tools pick the right schema within theoneOfbased on that key's value. supporting this would be a useful extension for@valibot/to-json-schemafor users targeting openapi, even thoughdiscriminatoris an openapi-specific keyword.