onChange reference can cause performance issues

[3nvi]

🚀 Feature request

Make onChange have the same reference between renders

Current Behavior

Currently, the onChange that gets "generated" and passed to a component via <Field> gets a difference reference on every render. Due to that, the component rendered from Field gets a prop with different reference on every render, which causes the component to be re-rendered regardless of the use of memo in it.

For example, let's suppose we have a Formik form which contains the following:

<Field as={Combobox} name="combobox" />

Any time any field gets rendered (not necessarily this one), the onChange prop that gets automatically passed to the Combobox gets a different reference. Thus, even if the Combobox implements a React.memo() it will have to re-render since its props changed.

On big forms or forms with a lot of styled components within them (a.k.a. loads of context consumers) this causes performance issues.

Desired Behavior

Ideally, we would want the onChange prop passed to components whose values have not changed to be referentially the same as the one in the previous render cycle. This way, the component which a memo won't render unnecessarily.

Suggested Solution

This issue doesn't happen with onBlur (which always keeps the same reference), but only with onChange. Seeing the code I see that it gets a new reference every time the executeChange function changes reference

formik/src/Formik.tsx

Line 545 in 44afb37

[executeChange]

.

The latter changes reference every time any of the values changes

formik/src/Formik.tsx

Line 483 in 44afb37

const executeChange = React.useCallback(

.
This is reference change happens only to cover the checkboxes case

formik/src/Formik.tsx

Line 521 in 44afb37

? getValueForCheckbox(getIn(state.values, field!), checked, value)

, so the state.values has been added to the useCallback array for something specific that doesn't necessarily apply to all forms out there.

Ideally we would want to handle this case differently. Perhaps separate the executeChange for the checkboxes case and keep the existing one for everything else?

To be fully honest, I might be missing something big here, so any info will be extremely helpful. At the end of the day, my goal is to have the Combobox (in the above example) not re-render when a value unrelated to it changes.

Who does this impact? Who is this for?

All users, but mainly projects with complex and/or heavy forms

P.S. Cheers for everything. Really appreciate the time you throw in this OSS.

[johnrom]

It looks like we should be able to use useEventCallback for all the handleX, since these functions aren't meant to be used during render. I could be misunderstanding, though, some of the intricacies of these hooks I'm not 100% on yet. Our implementation of useEventCallback doesn't accept [deps] like the React docs, and I'm not sure why.

https://reactjs.org/docs/hooks-faq.html#how-to-read-an-often-changing-value-from-usecallback

[donifer]

Having the same issue when using useFormik, the onChange changes references so all my inputs re-render on every change.

How are you guys working around this? I can't use onBlur (FastField). I also think onChange should be using useEventCallback internally.

[jaredpalmer]

Can someone submit a PR?

[jaredpalmer]

Yeah we should fix this. All handleX should use event callback.