-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
Support for conditional validation of fields? #282
Comments
Discriminated unions don't unify well with forms, and especially not with Superforms, as mentioned in the issues you link to. Since the schema data maps to a single form, what you say with a discriminated union is that you basically want two different forms, and handling that in the same That said, I don't think it's impossible to find a solution. If you can make a simple example of what you are trying to achieve, I can take a look. Use this Stackblitz repo as a template. |
By the way, does Felte handle this problem? In what way? |
@ciscoheat I think the difference between other libraries, such as Felte, and Superforms is that the others are fairly well decoupled from the often multiple validation libraries they support. Usually, form fields are registered as they are attached to form elements, through various mechanisms, and not by attempting to parse a provided schema for this purpose. In particular, Felte is based on the DOM, as basic HTML forms are aware of their constituent named inputs. Felte's dynamic forms document gives some clues to their implementation. However, Superforms' implementation at the most basic level seems to be tied directly to the parsing of a zod schema, and so it follows that basic functionality (such as mentioned in this ticket) can become as complicated to implement as zod itself, if not more. I can not comment whether the benefits of this approach outweigh its complexity, but I can say that I would gladly trade some extra type-safety or intellisense for actually being able to simply implement common UX patterns. I'm not quite sure the utility of creating a code example for this issue, as it is extremely basic, and I'm not sure how much you would want someone to attempt a Superforms-based solution within such an example. If it were simple to mock up an example in Superforms, I wouldn't have created this issue. I'll write out a basic situation, which should be reasonable for this context. Imagine this form:
This kind of switch is incredibly common, and this example is actually less basic than the one in that Felte document I had linked. If Superforms requires multiple forms (meaning extra boilerplate, logic duplication, etc) to manage this, and does not intend to support this in a more straight-forward and maintainable way in the near future, it would be good to know, so that we can make more informed decisions going forward. Please don't take this as criticism; I highly appreciate the work you and others have done here, and how responsive you are to the needs of your users. I have enjoyed working with Superforms so far, so my hope is that my colleagues and I can continue to use it as we begin to implement more dynamic forms. |
This is what I can come up with: https://stackblitz.com/edit/sveltekit-superforms-1-testing-abbsnr?file=src%2Froutes%2Fschema.ts,src%2Froutes%2F%2Bpage.svelte,src%2Froutes%2F%2Bpage.server.ts The key is that you have different schemas for the two entities, and validate them separately depending on the data. There are many ways of structuring the schemas, I'm making all common fields optional in the base schema and requiring them as needed in the subclassed ones. With that, it's as simple as using |
Also note that the baseSchema (with the optional fields) are used in the load function, otherwise the constraints and eventual default values won't be included. |
Thank you for this! This is a bit cleaner than our current implementation. I wasn't aware we could swap schemas using Our remaining issue is that we also have an API outside of SvelteKit that offers zod validations, which we would likely interact with in SPA-mode. Those validations use things like Do you have any knowledge of when |
Glad you found it useful! A Zod schema is fully composable, it's one of the few libraries that does that, so you could make a function that introspects a schema and extract the union sub-schemas. This is how extract and cache schema metadata, for example: https://github.com/ciscoheat/sveltekit-superforms/blob/main/src/lib/schemaEntity.ts#L87 That whole file should be of help if you want to get creative. :) And maybe this one as well if you want it to be type-safe, though it's a lot of work to get everything right. I understand that it's difficult if you rely on I don't know about any progress for |
It's really unfortunate if Superforms must treat schemas with unions/discriminated unions/ |
I'm still curious how Felte would handle that, without the types being dynamic in the end. |
The complete zod integration for Felte is in this file. Since it's basically only using zod for validation, it does not need to care about zod for other things, like registering fields in the form or errors object, etc. They hook into the browser's Form API for that, IIRC (that also may be a bit of an over-simplification). This is also why they're able to support multiple schema/validator libraries. |
Yes, that's merely error mapping, but then you have to handle everything else regarding schema metadata (default values, constraints, data coercion) elsewhere, both on client and server. I'm gonna make some experiments with discriminatedUnion, but I'm not optimistic. If the Zod creator even says that it was a mistake adding it, I'm hesitant to spend time on it. |
One of the down-sides with Felte is that you're on your own for transforming values/type coercion, and then you may lose some type safety when you transform values. Initial values are otherwise typed using a generic type variable, which could be inferred from the zod. As far as error reporting goes, they have several reporting strategies, one of them being an adapter to interface with the built-in Constraints Validation API. But I believe that's only for reporting, I don't think they actually use built-in browser validations internally when you provide a schema. |
So I've been experimenting, and the conclusion is that it's not possible to reconcile the error mapping of Superforms with Zod's discriminated unions. If you look at this schema data, which is how a discriminated union is mapped: {
name: string;
entity: { type: "person", DOB: Date } | { type: "corporate", taxId: string }
} With the Superforms error mapping, the "leaves" of this data tree is replaced with {
name: string[] | undefined;
entity: {
type: string[] | undefined,
DOB: string[] | undefined
} | {
type: string[] | undefined,
taxId: string[] | undefined
}
} See the problem here? The discriminator, the With I wish this was easily solvable, but alas, it's not. Mapping data and errors like this based on complicated branches of unions, it's a challenge. |
I'm having problems with this solution, The form returned on the load function is not the same type of the new validators it is throwing typescript errors whenever trying to change the options.validators property since it already has the baseSchema type |
Superforms v2 will handle this properly. |
Is your feature request related to a problem? Please describe.
There does not seem to be a clear way to manage conditional fields using Superforms. By conditional fields, I mean fields that are either displayed or not, and valid or not, depending on the value of another field.
This is usually managed in other libraries that rely on zod by utilizing
discriminatedUnion
, but this has been mentioned in at least two previous issues (here and here) that were promptly closed.It seems work on
discriminatedUnion
support has been deferred to after zod changes toswitch
, but there has been no movement (as far as I can see) onswitch
since it was mentioned nine months ago.Describe the solution you'd like
Any suggestions would be welcome, the least complex the better. My colleagues are starting to resent our technology choices 😓
Describe alternatives you've considered
Currently, we are having to work around this by using
refine
to handle conditional validations, but this is not only a bit convoluted, but we additionally have to add our own validation triggering logic becauserefine
doesn't seem to trigger until the rest of the schema is valid.We could try using Felte instead, but I'd rather not use that, for some other issues we've had with that library.
The text was updated successfully, but these errors were encountered: