jannikbuschke / formik-antd

Simple declarative bindings for Ant Design and Formik.
https://codesandbox.io/s/github/jannikbuschke/formik-antd-example
MIT License
591 stars 81 forks source link

FormItem errors aren't working for FieldArray's #3

Closed jwmann closed 5 years ago

jwmann commented 5 years ago

Using this example from Formik https://jaredpalmer.com/formik/docs/api/fieldarray

If we pass errors with the same way back as they are structured inside values.

e.g.: values:

{
  filters: [
    { name: undefined }
  ]
}

errors:

{
  filters: [
    { name: "Required!" }
  ]
}

FormItem doesn't catch the error. Mostly like because it's expecting a flat level key name.

jwmann commented 5 years ago

Just for comparison I can use <ErrorMessage name={filters[${index}].name} /> and it will display the correct error message properly.

jannikbuschke commented 5 years ago

I'm not really sure what you mean. Are you talking about the FormItem component?

jwmann commented 5 years ago

Yes. My mistake, I miswrote that.

When you pass the name prop from Formik, for a FieldArray it'll sometimes look like this filters[3].name but that key doesn't exist straight out in the errors or touched object. You need to traverse down the path using a lodash get() function or something similar.

When you use the <ErrorMesage name={} /> component that comes with Formik, it does this automatically.

jannikbuschke commented 5 years ago

Ok I see. Currently FormItem and handling FieldArrays in general is totally work in progress. I didn't yet have time to get my head around a proper solution. (only the components in the README are expected to be kind of stable).

I will address validation and lists shortly.

jwmann commented 5 years ago

Your implementation is pretty close.

If you use lodash get() you could fix this easily and maintain normal functionality. https://lodash.com/docs/4.17.11#get

var object = { 'a': [{ 'b': { 'c': 3 } }] };

_.get(object, 'a[0].b.c');
// => 3

For instance, you have this

  <Field name={name}>
    {({ field, form }: { field: any; form: FormikProps<any> }) => {
      const { name } = field;
      const hasError = form.errors && form.errors[name] && form.touched[name];
      return (
        <Form.Item
          label={props.label}
          validateStatus={hasError ? "error" : undefined}
          hasFeedback={false}
          help={
            hasError
              ? (form.errors[name] as any).map((error: string) => (
                  <li>{error}</li>
                ))
              : undefined
          }
>

You could do something like this

  import get from "lodash/get";

  <Field name={name}>
    {({ field, form }: { field: any; form: FormikProps<any> }) => {
      const { name } = field;
      const { errors = {}, touched = {} } = form;
      const error = get( errors, name, undefined );
      const isTouched = get( touched, name, false );
      const hasError = error !== undefined && isTouched;
      return (
        <Form.Item
          label={props.label}
          validateStatus={hasError ? "error" : undefined}
          hasFeedback={false}
          help={hasError && <li>{error}</li>}
>
jwmann commented 5 years ago

Apologies for closing and re-opening. I pressed a GitHub keyboard shortcut by accident.

jwmann commented 5 years ago

This FormItem is being a real blocker for my project, I really need the above to be fixed soon. I'd make a PR myself but I have no way to test locally.

jannikbuschke commented 5 years ago

You can clone this repository https://github.com/jannikbuschke/formik-antd-playground with the --recursive flag and have a good local test experience. Pls follow the README to get started (it should be very simple)

jwmann commented 5 years ago

@jannikbuschke This is awesome! Thank you. Expect a PR probably on Monday 🙌

jannikbuschke commented 5 years ago

Sounds good! Can you give me some idea about how you would use the FormItem inside a FieldArray? Like some pseudo code with a FieldArray, FormItem and Input component. I would expect the FormItem to wrap an Input component, is this correct? (I am not yet sure how to handle FieldArrays properly, and am interested in any input)

jwmann commented 5 years ago

Sure thing.

From https://jaredpalmer.com/formik/docs/api/fieldarray

 <FieldArray
    name="listOfFriends"
    render={arrayHelpers => (
      <div>
        {values.friends.map((friend, index) => (
          <div key={index}>
            <Field name={`listOfFriends[${index}].name`} />
            <Field name={`listOfFriends.${index}.age`} /> // both these conventions do the same
            <button type="button" onClick={() => arrayHelpers.remove(index)}>
              -
            </button>
          </div>
        ))}
        <button
          type="button"
          onClick={() => arrayHelpers.push({ name: '', age: '' })}
        >
          +
        </button>
      </div>
    )}
  />

The FieldArray component just acts as a helper component to render fields based on an index. It provides arrayHelpers that allow to push() or remove() from the field listOfFriends which is an array of fields or array of groups of fields.

So the FieldArray component itself you don't have to worry about, the only thing you need to account for is the naming convention which makes FieldArray's possible.

<Field name={`listOfFriends[${index}].name`} />
<Field name={`listOfFriends.${index}.age`} /> // both these naming conventions do the same thing

Now Formik's name prop handles these names which output's string like this: listOfFriends[5].name which would be the name field inside the 5th index of the array from the form key listOfFriends.

Now this all works completely fine using your package because all we're doing is passing the name prop to <Field /> and then Field handles the rest.

However; inside <FormItem /> we're doing a bit more than that. We're manually checking the errors, and touched object to see if it exists and if it does, passing them manually to <Form.Item /> which works fine for fields that have a flat name like <Input name="fieldName" /> but for listOfFriends[5].name we need to do more than that, and must traverse down both objects.

This is literally the design of what lodash/get does 😃

jannikbuschke commented 5 years ago

Ok sounds good to me. Also adding lodash as a dependency (I guess if importing correct, mainly only the get function will be bundled) is fine for me.

jwmann commented 5 years ago

For some reason, using formik-antd-playground. name always appears as "" which is really strange. Do you know why that could be?

~~the name inside the field object is also "" however props.name is actually the right name~~

~~I made a component on the app side minus the Form.Item part and it seems to be a Formik problem? Or maybe some kind of typescript issue that I'm not aware of. Basically, I can pass name to Field and Field's props name remains ""~~

Seems as though the problem was in the original name variable after all. name was being passed but not destructured from props. I was confused because I'm not used to typescript so I assumed props: { name: string; children: React.ReactNode } & FormItemProps was destructuring it, but it wasn't. So name was just an undeclared variable. No idea why that didn't error on compile.


Also sidenote, the default version of formik-antd that formik-antd-playground installs is 0.8.2. I had to manually pull in the latest version.