iway1 / react-ts-form

https://react-ts-form.com
MIT License
2.01k stars 33 forks source link

enum support alternative? #63

Open scamden opened 1 year ago

scamden commented 1 year ago

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?

scamden commented 1 year ago

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,
    },
  }}
 />
iway1 commented 1 year ago

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
scamden commented 1 year ago

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)

KubaJastrz commented 1 year ago

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.

scamden commented 1 year ago

does this PR (https://github.com/iway1/react-ts-form/pull/86) mean we can close this issue as resolved?

scamden commented 1 year ago

ah i see now that this was a removal of only one of the warnings. @iway1 could we get rid of printUseEnumWarning as well? or was this something different.

iway1 commented 1 year ago

@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 SelectField component with useEnumValues, and then later you need to use that component with dynamic values (IE you fetch the select options from the server), the SelectField you've built is no longer going to work.

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