-
Notifications
You must be signed in to change notification settings - Fork 40
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
enum support alternative? #63
Comments
let me color this in with an example of why i really want enums in the schema. here's what i have to do to get around this right now: const baseEditSchema = z.object({
scope: z.number(),
category: z.number().nullable(),
minimumFidelity: z.string(),
additionalNotesMd: z.string().nullable(),
explainerMd: z.string().nullable(),
});
// NOTE: have to create a second schema to "cast" back to the real types for enums
const inputSchema =
baseEditSchema.extend({
minimumFidelity: z.nativeEnum(FidelityEnum),
});
const FIDELITY_LIST_OPTIONS = getListOptionsFromValues(
Object.values(FidelityEnum)
);
<ZodForm
schema={baseEditSchema}
onSubmit={async (values) => {
await onSubmit(
// NOTE: i have to do a second unnecessary parse here to do a safe conversion from `string` to `FidelityEnum` which my update requires
inputSchema.parse(
values
)
);
onClose();
}}
props={{
minimumFidelity: {
listOptions: FIDELITY_LIST_OPTIONS,
},
}}
/> |
I think this can work as long as you still passed the enum values as a prop. The big issue with my initial implementation was the fact that I was extracting the enum values from the zod schema itself which created a mess, so yeah I'll mark it as not deprecated but will be removing the ability to access the enum values in the component itself other than explicitly passing props. it is kind of awkward to have to do this in the mapping but IDK how else to support enums: const mapping = [
[z.enum(['fakeValue']), EnumComponent]
] as const |
ya that mapping is kinda awk but maybe you could just expose a util like with createUniqueSchema? can i ask what's messy about extracting the enum values? (seems really convenient) |
How about something like this? const schema = z.object({
color: z.enum(['green', 'red', 'blue']),
fruit: z.enum(['apple', 'pear', 'banana']),
});
const mapping = [
[schema.shape.color, SelectField],
[schema.shape.fruit, SelectField],
] as const;
const Form = createTsForm(mapping);
function SelectField() {
const { field } = useTsController<string>();
const options = useEnumValues();
return ...
} It seems to work just fine. |
does this PR (#86) mean we can close this issue as resolved? |
ah i see now that this was a removal of only one of the warnings. @iway1 could we get rid of |
@scamden I just ran into some issues that sort of felt like footguns when I tried using enums myself and didn't want to lead anyone down the wrong path with them. For example if you build a That was my reasoning for wanting to get rid of the feature, didn't want to confuse people. But maybe it's OK to keep it. It is ultra-convenient when it works, and it's possible to build a small wrapper component when running into the issue I described above. Might be best to keep the feature and document how to deal with potential issues? Add a new enums doc page and then describe how to deal with cases where you need the values to be dynamic. I think there are still some other issues with this though that need to be worked out, I remember running into some issues w/ createUniqueFieldSchema when messing with it but I can't remember exactly what it was. Gonna play with this some though. I'll go ahead and make a release with the warning removed |
I noticed you deprecated enum support with the reasoning the strings() can do everything enums can. The problem I'm finding is that the values I get back from the form on submit have strings in their type instead of enums (as you'd expect). I'm using your workaround to pass select options to the component but what's the workaround to get the types to be correct?
Our GQL input mutations have actual enums so i'd love to be able to define a typed form that matches that type exactly. Is there an alternative solution I just haven't found?
The text was updated successfully, but these errors were encountered: