jaredpalmer / formik

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

FieldArray remove helper can set error state to empty array #784

Open TLadd opened 6 years ago

TLadd commented 6 years ago

Current Behavior

When using FieldArray's remove helper, my errors state for the field gets set to an empty array for two renders. This is problematic if you are expecting errors to be an object with only string values.

This line is what sets the errors state to an empty array, since the function is used both for the form state as well as the errors state.

Steps to Reproduce

Use FieldArray with a populated array and call .remove() with an index. Log the errors state and observe that for two renders, the field has an empty array error state.

Here's an example codesandbox: https://codesandbox.io/s/p9x78o5zyq If you open up the console and click the X button to remove one of the items in the array, you'll notice that errors.arr is set to an empty array for two logs before being reset by validate.

Imgur

Expected behavior

The arr field should not be present in the errors object for these cases

Suggested solution(s)

Need a way to not default the error state to an empty array on remove. Seems like it might be necessary to change updateArrayField to allow for different logic when updating the error state rather than sharing the function that updates the form state.

CodeSandbox Link: https://codesandbox.io/s/p9x78o5zyq


stale[bot] commented 6 years ago

Hola! So here's the deal, between open source and my day job and life and what not, I have a lot to manage, so I use a GitHub bot to automate a few things here and there. This particular GitHub bot is going to mark this as stale because it has not had recent activity for a while. It will be closed if no further activity occurs in a few days. Do not take this personally--seriously--this is a completely automated action. If this is a mistake, just make a comment, DM me, send a carrier pidgeon, or a smoke signal.

stale[bot] commented 6 years ago

ProBot automatically closed this due to inactivity. Holler if this is a mistake, and we'll re-open it.

ersel commented 5 years ago

This issue seem to occur on formik@2.0.1-rc.5

After removing a field with the arrayHelper.remove, errors object contains an empty array which makes isValid field return false as well.

ersel commented 5 years ago

As a temporary fix I ended up calling validateForm manually after a certain timeout.

onClick={() => {
  arrayHelpers.remove(index);
  setTimeout(() => {
    validateForm();
  }, 200);
}}
TLadd commented 5 years ago

Yeah, after updating to formik@2.0.1-rc.5, it actually looks like now the empty array remains there, whereas before it would go away on subsequent renders.

Updated codesandbox: https://codesandbox.io/s/formik-example-uez7v isValid having an incorrect value seems like an issue.

3nvi commented 5 years ago

This should be re-opened as it's still happening on rc.13

TLadd commented 5 years ago

I was fine with this being closed because the error changed to be somewhat worse in the newer versions of Formik, and so I opened https://github.com/jaredpalmer/formik/issues/1616. We probably don't need both open.

samueldepooter commented 4 years ago

As a temporary fix I ended up calling validateForm manually after a certain timeout.

onClick={() => {
  arrayHelpers.remove(index);
  setTimeout(() => {
    validateForm();
  }, 200);
}}

This was a good start but it causes a timeframe where an error does exist (and in my case that would show an error block for a split second).

For my use-case, which involves only validation on submit, I fixed it the following:

// get a copy of your old errors so that they're not reset
const errors = Object.assign({}, formikBag.errors);
// remove it (triggers the validation which causes the mentioned issue)
helpers.remove(field.index);
// set your errors back to your old errors
formikBag.setErrors(errors);
klimashkin commented 4 years ago

It also doesn't always make field touched as advertised. Due to multiple bugs with FieldArray it's easier to explicitly use setFieldTouched/setTouched and setFiledValue/setValue

seamuslowry commented 4 years ago

Was this fixed by https://github.com/jaredpalmer/formik/pull/2115?

alex-sh-li commented 4 years ago

had the same problem on 2.0.6, upgraded to 2.2.0 problem still existed.