jaredpalmer / formik

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

[v2] removeItem puts empty array in in error state and causes isValid to be incorrect #1616

Open TLadd opened 5 years ago

TLadd commented 5 years ago

🐛 Bug report

Current Behavior

When I call FieldArray's removeItem, it sets the error state to an empty array, and it stays that way until validate is run again. Having a key with a truthy value results in isValid being calculated as true, when the form is actually still valid.

This codesandbox illustrates the issue: https://codesandbox.io/s/formik-example-uez7v. When you click the X next to one of the items, the errors state is Object {arr: Array[0]} and isValid is false.

Somewhat of a duplicate of https://github.com/jaredpalmer/formik/issues/784, but the behavior has changed to be a bit worse. Before, this was just a blip that ended up being cleaned up on a future render, presumably because validate was being triggered immediately and overwriting the error state. Now, no such rerunning of validation is occurring.

Expected behavior

I would expect that the error state for array fields isn't automatically set to [], which would then also solve the isValid problem.

Reproducible example

https://codesandbox.io/s/formik-example-uez7v.

Your environment

Software Version(s)
Formik v2.0.1-rc.5
React 16.8.6
TypeScript N/A
Browser Chrome
npm/Yarn N/A
Operating System OS X
sibelius commented 5 years ago

I can confirm this wrong behavior

bichoymessiha commented 5 years ago

As mention in this https://github.com/jaredpalmer/formik/issues/784#issuecomment-503135849. You need to validate manually after removing item.

lightitnow commented 5 years ago

This happens to me also, regardless of validateOnChange being set or not. I tried validating manually after removing item, but the error state returns the next time I add and remove an item.

sibelius commented 5 years ago

simple helper to fix this until the proper fix lands

type SetFieldValue = (field: string, value: any, shouldValidate?: boolean) => void;
export const arrayHelpersRemove = <T extends any>(
  name: string,
  values: object,
  index: number,
  setFieldValue: SetFieldValue,
) => {
  const arr = values[name];

  const newArr = [...arr.slice(0, index), ...arr.slice(index + 1)];

  setFieldValue(name, newArr);
};

arrayHelpersRemove('list', formik.values, i, formik.setFieldValue);

it could be turned in a hook

lightitnow commented 5 years ago

@sibelius Thanks! That worked, but arrayHelpers.remove does more than that right? Btw, could PR #1782 be a fix for this?

inv8der commented 4 years ago

Hey! I worked on #1782. The issue described here is exactly what prompted me to investigate and submit that PR. I found similar issues with some of the other FieldArray methods, as well. I also discovered validateOnChange was not working, so I tried to fix that too, though my solution required updating the setFormikState typedef.

KrzysztofMadejski commented 4 years ago

Apart from cleaning empty array on errors there is one more issue.

If only some objects in the array had errors, ie.

{"errors": {"friends": [null, null, "invalid friend"]}}

then removing the field with error leaves:

{"errors": {"friends": [null, null]}}

@inv8der to solve this you might need to change line: https://github.com/jaredpalmer/formik/pull/1782/files#diff-7d037a4310f1022dee96554929bd14b3R275

copy.filter(err => err !== '' && err !== null).length > 0 ? copy : undefined;

Thanks for the PR!

imdadul commented 4 years ago

I am also having the same problem. Waiting for the update.

gmltA commented 4 years ago

The same problem is applicable to 'touched' state of array field. Field state gets removed from 'touched' object as soon as the last item is removed. That's not right.

asvny commented 4 years ago

+1 .. I am also facing the same problem.

tyteen4a03 commented 4 years ago

+1

h4t0n commented 3 years ago

+1

lexanth commented 3 years ago

I've worked out what's going on:

As a workaround, you can set validateOnChange to true for a FieldArray - then it will automatically tidy up the broken generated errors by triggering a revalidate (which is what @sibelius's fix was doing).

I had a further issue here in that that only works when the values have actually changed, but you can trigger this by calling move with the source and destination index the same, so I had to ignore null moves.

The proper fix would be ignore updating the errors or touched state if they're undefined (while retaining the validateOnChange behaviour). If the touched state is undefined, there can't be anything to shift around.

i.e. something like

      const prevErrors = getIn(prevState.errors, name)
      let fieldError = alterErrors && !!prevErrors
        ? updateErrors(prevErrors)
        : undefined;
      const prevTouched = getIn(prevState.touched, name)
      let fieldTouched = alterTouched && !!prevTouched
        ? updateTouched(prevTouched)
        : undefined;
sergei-maertens commented 12 months ago

Looking at https://github.com/jaredpalmer/formik/discussions/3526 and seeing that there are recent patch releases, what would be needed to address these bugs and get them released?