Add new issue reason for non-matching variant key #277
Conversation
✅ Deploy Preview for valibot canceled.
|
|
|
||
| /** | ||
| * Variant option type. | ||
| */ | ||
| export type VariantOption<TKey extends string> = | ||
| | ObjectSchema<Record<TKey, BaseSchema>, any> | ||
| | ObjectSchema<Record<TKey, LiteralSchema<Literal>>, any> |
There was a problem hiding this comment.
If I understand this line correctly, only literals can be passed as values in the object variants now. This would make variant useless in my eyes. Did I miss something?
There was a problem hiding this comment.
Yes, as discussed here. Why do you think it would make variant useless? I have a hard time finding real-world use-cases with anything else than literals. Zod also doesn’t support much more, see here. Although Zod supports some special cases like literals that are wrapped in ZodLazy or ZodEffects as well as null, undefined and enums.
There was a problem hiding this comment.
If I understand it correctly, each key of the objects must now have a literal as its value. However, usually only the discriminator is a literal and the rest is some other data type. For example, is the following code still possible after your changes?
const Schema = variant('type', [
object({ type: literal('a'), a: string() }),
object({ type: literal('b'), b: object({ key: string() }) }),
]);There was a problem hiding this comment.
Ah now I get your point. I need to take a closer look at VariantOption and ObjectSchema. Because from just reading the code I come to the same conclusion as you. But for some reason the example you provided works fine. See also the unit tests I’ve added in this PR, the val property uses non-literal schemas and there is no typescript compilation error... 🤔
There was a problem hiding this comment.
Yes, I had thought of that and wondered about it. What is the reason why we can only accept literals as discriminators? Presumably you have to access the .literal for the error info, right?
There was a problem hiding this comment.
What is the reason why we can only accept literals as discriminators? Presumably you have to access the
.literalfor the error info, right?
Exactly. An alternative solution would be to just pass the schema objects of every option to the issue requirements instead of limiting the schemas to literal schemas. In this case the consumer needs to decide what to do with the requirements and could extract the .literal value in case the schema object is a literal schema.
There was a problem hiding this comment.
I will think about it. However, I probably won't be able to work on this PR for another three to four weeks.
There was a problem hiding this comment.
I’ve just pushed another commit which explores the idea of passing the variant options as issue.requirement.
However, I probably won't be able to work on this PR for another three to four weeks.
No worries. There is zero pressure from side. If I can help you out with anything just let me know 😉.
|
This is partially fixed in v0.25.0. Sorry that I did not build on this PR to implement it. |
|
I think this is now completely fixed with the changes in v0.28.0 |
Fixes #235.
This is a breaking change because with this change only literal schemas are allowed for the discriminator.