jaredpalmer / formik

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

Multiple calls to setFieldValue inside async action result in form errors #3754

Open matt-sanders opened 1 year ago

matt-sanders commented 1 year ago

Bug report

Current Behavior

We have a use case where fields need to be updated as the result of an async function, very similar to this example. However, we would like to update multiple fields. We find that when we use setValues things work as expected, however when we make multiple calls to setFieldValue it results in an error.

This doesn't happen if the calls to setFieldValue happen as an immediate callback ( e.g. from a button ) but when the callback happens after an async action, things go a little awry.

The obvious, initial solution is to just use setValues, but we may want to update these fields individually given certain circumstances ( e.g. if x, then update y but not z )

Expected behavior

Multiple calls to setFieldValue should not trigger validation errors when the form is valid.

Reproducible example

Live example: https://d4pe7b.csb.app/

Codesandbox: https://codesandbox.io/s/proud-wind-d4pe7b?file=/src/App.js

Your environment

Software Version(s)
Formik 2.2.9
React 18.2.0
TypeScript -
Browser Chrome
npm/Yarn npm
Operating System MacOS
3nvi commented 1 year ago

Yup, validated this today as well.

It's because each setFieldValue call doesn't take into account any updated values through other calls.

Thus if you have 2 back-to-back setFieldValue , they both use the same snapshot of values without the 2nd setFieldValue factoring in the updates of the 1st setFieldValue call.

LaikaTheSpaceDog commented 1 year ago

I was having a similar issue and can confirm that my bug was caused by what @3nvi has described above 👍

matt-sanders commented 1 year ago

Ok, so is there any solution to that? I would have thought that awaiting the promise from setFieldValue should cause the next one to use the correct values. Kind of like how setState would work when passing a function as the callback.

VizTheWiz commented 5 months ago

Encountered this exact issue today, with version 2.4.5, on chrome / Ubuntu / react 18.2. At least I was able to fix this with setValues, but I would have expected that awaiting a setFieldValue call would ensure the state is correctly updated upon continuation.

abhinavkumar940 commented 5 months ago

Can confirm this on v 2.4.6 as of today. Also as @VizTheWiz suggested, setValues helps.

CodeSmith32 commented 2 months ago

Yep, this is still an issue. Kind of frustrating too. I spent awhile assuming I was doing something wrong.


Ok, so I dug around. The issue is due to this line:

https://github.com/jaredpalmer/formik/blob/main/packages/formik/src/Formik.tsx#L596

const setFieldValue = useEventCallback(
  (field: string, value: any, shouldValidate?: boolean) => {
    dispatch({
      type: 'SET_FIELD_VALUE',
      payload: {
        field,
        value,
      },
    });
    const willValidate =
      shouldValidate === undefined ? validateOnChange : shouldValidate;
    return willValidate
      ? validateFormWithHighPriority(setIn(state.values, field, value)) // ** L596 ***********
      : Promise.resolve();
  }
);

The issue occurs because state.values is used, not stateRef.current.values. When the preceding call to dispatch is triggered, it updates the stateRef, but this does not synchronously update state. Because of this, two synchronous calls to setFieldValue are verified independently, and only the latter takes effect.

By way of example:

Say a form has two boolean fields, "foo" and "bar", they both must be true to be considered valid, and they both start off as false.

Given a callback that sets both to true at once...

// values == { foo: false, bar: false }
setFieldValue("foo", true);
setFieldValue("bar", true);

The first call, for "foo", takes the current state, sets just "foo" to true, and then tests for validity.

{ foo: true, bar: false }

This of course won't be valid.

The second call, for "bar", however, doesn't take the updated state (stateRef.current.values), it takes the old state (state.values), and updates "bar":

{ foo: false, bar: true }

This is also not valid.

But by this time, the function ends. Even though stateRef.current is correct and has up-to-date values for both properties, it itself was never tested for validity.

Thus, using something like the following in setFieldValues instead could fix this issue:

    return willValidate
      ? validateFormWithHighPriority(stateRef.current.values) // ** L596 ***********
      : Promise.resolve();

But, my concern is, it seems pretty strongly explicit that state.values get updated separately from stateRef.current.values and passed to validateFormWithHighPriority vs. just using the latest version found in stateRef.values.current.

So, what was the intention here? And how can we fix this issue without causing any other issues that this line was possibly hoping to avoid?

CodeSmith32 commented 2 months ago

Digging further, this line of code has used setIn since it was introduced:

Commit Diff: https://github.com/jaredpalmer/formik/commit/123e3925a02ae952c445daef072021224ab3690a#diff-a2d99b5c519f468d81549cc342ecb0fb4096b285ee93e36a250e4fcccd963565R472

Commit File: https://github.com/jaredpalmer/formik/blob/123e3925a02ae952c445daef072021224ab3690a/src/Formik.tsx#L472

But at that time, there was no stateRef, leading me to wonder if maybe jaredpalmer had to use setIn since it would've been the only way to get the latest version of the state. Because, at that point, dispatch was from a React reducer, which doesn't provide up-to-date state till the next render.

@jaredpalmer, is this true? Is it safe now to use stateRef.current.values as I suggested in my previous comment? Or is there another reason you had used setIn when passing the state to validateFormWith*Priority?