Skip to content
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

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

Open
dhottmann opened this issue Jul 23, 2019 · 10 comments
Open
Labels

Comments

@dhottmann
Copy link

dhottmann commented Jul 23, 2019

🐛 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
Copy link

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
Copy link

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

@lightwave
Copy link
Contributor

lightwave commented Sep 7, 2019

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
Copy link
Collaborator

johnrom commented Sep 7, 2019

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

@stale stale bot added the stale label Nov 6, 2019
@davidroeca
Copy link

This is still an issue for me.

@ml054
Copy link

ml054 commented Nov 19, 2019

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
Copy link
Collaborator

johnrom commented Nov 19, 2019

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.

@stale stale bot added the stale label Jan 19, 2020
@davidroeca
Copy link

Still relevant

@stale stale bot removed the stale label Jan 22, 2020
@stale stale bot added the stale label Mar 22, 2020
@use-strict
Copy link

Will this issue be fixed?

@stale stale bot removed the stale label Mar 27, 2020
@stale stale bot added the stale label May 30, 2020
@karlvonbonin
Copy link

Not fixed yet..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants