Form does not rerender when formContext functions change

[robbieculkin]

Prerequisites

• I have searched the existing issues
• I understand that providing a SSCCE example is tremendously useful to the maintainers.
• I have read the documentation
• Ideally, I'm providing a sample JSFiddle, Codesandbox.io or preferably a shared playground link demonstrating the issue.

What theme are you using?

other

Version

5.19.3

Current Behavior

Form's current shouldComponentUpdate uses a custom deep equality check that assumes all functions are equal.
This assumption was added for performance reasons, see #255.

This assumption breaks function closures, meaning callback functions are fired from within the form with stale state. See SSCCE example in "Steps to Reproduce".

Expected Behavior

Callback functions are a common React pattern, used to provide dynamic behavior depending on the state of the application. This state is usually encoded in the callback's closure. Any component that takes such a function as a prop should update when the function changes.

A possible solution would be to switch shouldComponentUpdate to use shallow equality, Object.is(), which is the standard for most React components.

To address the performance concerns, the typical React pattern to avoid unnecessary component rerenders is to wrap any callback function in a useCallback.

Steps To Reproduce

Sandbox: https://codesandbox.io/p/sandbox/serene-noether-dx4x7y

Minimal example that shows stale values displayed from a custom field template. Click "increment counter" button, observe the difference in value between calls to getCounterValue inside & outside the form.

Environment

Anything else?

No response

[heath-freenome]

@robbieculkin Interesting use case... And points out that we need to be more careful with comparing callbacks in our form props, since those may also change. I hesitate to just switch out the comparison props via isEqual() instead of deepEqual due to potential performance degradations... Part of me is considering updating deepEquals to accept an optional whiteList of paths to functions within an object that will be compared via Object.is() (i.e. fall-thru through in the function). Then we would always whitelist the callback props in FormProps and provide a new prop to add additional whitelist function paths, which would solve your issue. Are you interested in building this new feature?

[robbieculkin]

@heath-freenome Understood that those clients who aren't memoizing their callback functions might run into performance issues if we switched from deep to shallow equality.

whitelist is an interesting idea, but do you think it might be a bit simpler if we started by exposing a boolean flag that replaces prop comparisons from RJSF deepEquals with shallow equality across the board? We could default this to false to start, but perhaps eventually (as part of a major version bump) default it to true? This might be a kinder way to migrate clients towards best practices in the long run. I'd be interested to implement this boolean switch.

[heath-freenome]

@robbieculkin That also seems like a decent (and simpler) solution. You'd have to update both Form as well as the @rjsf/utils function shouldRender

[robbieculkin]

I've got an implementation ready here: #4702