jaredpalmer / formik

Build forms in React, without the tears 😭
https://formik.org
Apache License 2.0
34k stars 2.79k forks source link

Typescript compile errors with many FormikHelpers when using nested fields #1698

Open dhottmann opened 5 years ago

dhottmann commented 5 years ago

🐛 Bug report

On a project that uses Typescript and nested fields, the defs are not flexible enough for many functions in FormikHelpers.

interface Values {
  some: {
    nested: string;
  };
}

const MyEnhancedForm = withFormik({
  mapPropsToValues: (): Values => ({
    some: { nested: "" }
  }),
  handleSubmit: () => {}
})(props => {
  const { setFieldValue } = props;
  return (
    <form onSubmit={props.handleSubmit}>
      <Field name="some.nested">
        {({ field }: FieldProps<string>) => {
          return (
            <input
              type="text"
              {...field}
              onChange={event =>
                setFieldValue("some.nested", event.target.value)
              }
            />
          );
        }}
      </Field>
      <button type="submit">Submit</button>

      <DisplayFormikState {...props} />
    </form>
  );
});

In the above example, the #setFieldValue() call is invalid bc the function is defined using (field: keyof Values & string, ... Obviously some.nested doesn't exist as a key of Values, so it doesn't compile.

Expected behavior

Nested keys should be allowed as a field key in FormikHelpers functions.

Reproducible example

https://codesandbox.io/s/typescript-formikhelpers-defs-dont-work-for-nested-values-9r5nm

Suggested solution(s)

The field key either needs to be relaxed to string or figure out some other Typescript magic to include the nested field names.

Your environment

Software Version(s)
Formik 2.0.1-rc.12
React 16.8.6
TypeScript 3.5.1
Browser Chrome 75.0.3770.142 (latest)
npm/Yarn npm 5.6.0
Operating System MacOS 10.14.5
bricejar commented 5 years ago

The current design of Typescript would not allow to make this type safe. Therefore I agree that the key needs to be relaxed to string. We are hardcoding the path as a string DSL, we should use an other typed model for the path.

bricejar commented 5 years ago

It is easy to work around this by updating the value of the root field and manually updating the nested field :)

lightwave commented 5 years ago

It is easy to work around this by updating the value of the root field and manually updating the nested field :)

Although it could be worked around like that, but it defeats the purpose of having TS check for the field name. Type checking only the top level field name just gives us a false sense of safety.

Nested field is one example the "hardcoding" field check will break. Another example is dynamic field name. Both cases break the typing of setFieldValue which expects a fixed set of field names. IMHO, field name is not an enumeration all the time. For a generic library like Formik to assume field name to be an enumeration type is too restrictive.

I work around both problem by using any type.

const formik = useFormikContext<any>();
formik.setFieldValue(field.name, someValue);

Here field.name is some path such as 'user[0].phone'

So, I think relaxing the type of field to string is the right call.

johnrom commented 5 years ago

Wait when did this keyof get merged in?! I did tons of experiments with this and the only way I found to "strongly type" it was a proxy (this is done over v1, planning to re-do over v2 though).

Basically, you can provide a wrapper for Field that has the name filled out for you, but it cannot be strongly typed per se. Below it is used like

const MyForm = () => <Formik>
  {fields => {
    const NameField = fields.name.first._getField();
    return (
        <NameField /> // name="name.first"
    );
  }}
</Formik>

1336 #1334 https://github.com/johnrom/formik-typed

davidroeca commented 5 years ago

This is still an issue for me.

ml054 commented 5 years ago

optionally we might use additional layer which will make props checked like that:

export function formFieldsAccessor<TForm>() {
        return function nested<
        K extends keyof TForm, 
        L extends keyof TForm[K] = keyof TForm[K], 
        M extends keyof TForm[K][L] = keyof TForm[K][L]
        >(prop1: K, prop2?: L, prop3?: M): string {
        return prop1
            + (prop2 ? "." + prop2 : "")
            + (prop3 ? "." + prop3 : "");
    }
}

Then:

export const loginDtoFields = formFieldsAccessor<LoginDto>();

And usage:

image

As result field type set to string is no longer a problem.

I've managed to solve this by little hack:

export function formFieldsAccessor<TForm>() {
    return function nested<
        K extends keyof TForm, 
        L extends keyof TForm[K] = keyof TForm[K], 
        M extends keyof TForm[K][L] = keyof TForm[K][L]
        >(prop1: K, prop2?: L, prop3?: M): keyof TForm & string {
        return prop1
            + (prop2 ? "." + prop2 : "")
            + (prop3 ? "." + prop3 : "") as keyof TForm & string;
    }
}

Now I have strongly typed fields access: image

It involves extra layer, but proxy is not needed.

johnrom commented 5 years ago

The proxy above does more than simply return the name, it returns a strongly typed Field compliant with your interface, meaning your value, validate, and onChange functions can be strongly typed. It also paves the way to add onFormat = (value: TValue) => string and onParse = (value: string) => TValue props to get rid of the pseudo-typing that exists within Formik.

davidroeca commented 4 years ago

Still relevant

use-strict commented 4 years ago

Will this issue be fixed?

karlvonbonin commented 3 years ago

Not fixed yet..