rjsf-team / react-jsonschema-form

A React component for building Web forms from JSON Schema.
https://rjsf-team.github.io/react-jsonschema-form/
Apache License 2.0
14.1k stars 2.18k forks source link

Custom Field onChange not working #3367

Open LouaiCoolshop opened 1 year ago

LouaiCoolshop commented 1 year ago

Prerequisites

What theme are you using?

core

Version

4.2.0

Current Behavior

In my react app, I have a form with a few fields. Two of them use a custom field. What I'm trying to do is that when the 1st one's value changes, the 2nd one should clear.

To do that, I'm passing all the form's data into the form context, and making the 2nd field check when the 1st field's value changes (using useEffect), and call "onChange(null)".

The problem is that sometimes the 2nd field's value doesn't nullify. It would call "onChange(null)" but the "formData" for it does not update, and the field will stay the same.

But, if I put that "onChange(null)" into a setTimeout for 50ms, it works always.

The custom field (simplified):

function AutoCompleteAsync(props: AutocompleteAsyncFieldProps) { // type extends "FieldProps"
    const {
        id,
        formData,
        onChange,
        customOptions, // [mine] custom options passed
        isDependant, // [mine] custom boolean passed to show if this field depends on another (resets when it changes)
        formContext,
    } = props;

    const [localValue, setLocalValue] = useState<string>('');
    .... other unrelated states ....

    const allFormData = useMemo<Record<string, any>>(
        () => formContext?.allFormData || {},
        [formContext],
    );

    // If this field depends on another field, evaluate the dependance field name and value
    const [dependsOn, dependsOnEntity, dependanceValue] = useMemo<
        [string | null, string | null, string | null]
    >(() => {
        if (isDependant) {
            // "dependsOn" might contain dots (eg "status.key"), so extract "status.key" from allFormData
            const extractedValue: string | null = get(
                allFormData,
                customOptions.asyncData!.dependsOn!,
                null,
            );
            return [
                customOptions.asyncData!.dependsOn!,
                customOptions.asyncData!.dependsOnEntity!,
                extractedValue,
            ];
        }
        return [null, null, null];
    }, [isDependant, customOptions, allFormData]);

    const onValueSelected = useCallback(
        (val: string) => {
                setLocalValue(val);
                onChange(val);
        },
        [onChange],
    );

    useEffect(() => {
        setLocalValue(formData);
    }, [formData]);

    // In case this field depends on another, reset it when the dependent field value changes
    useEffect(() => {
        if (isDependant) {
            // !!! HERE IS THE PROBLEM: setTimeout as workaround to make it work
            setTimeout(() => {
                onChange(null);
            }, 50);
        }
    }, [
        isDependant,
        dependanceValue,
    ]);

    return (
        <Autocomplete
            ...
            value={localValue}
            onChange={(ev, newVal) => {
                setLocalValue(newVal);
                onChange(newVal);
            }}
        />
    );
}

export default AutoCompleteAsync;

I'm certain that the "onChange(null)" is being called when I need it. Even used the debugger and stepped in. But sometimes it doesn't update the field formData to null when I don't have that setTimeout.

Expected Behavior

When I call "onChange(null)", it should always update the formData instead of just sometimes, without needing to use a setTimeout.

Steps To Reproduce

  1. Inside a react app
  2. Initialize a form with 2 custom fields
  3. Pass in all the form's data into the RJSF component's form context
  4. In the second custom field, add a useEffect to the form data's 1st field
  5. In that useEffect, call "onChange(null)" when the 1st field's value changes
  6. The second field will not update to null sometimes.

Environment

- OS: Windows 11 Pro
- Node: 18.0.0
- npm: 8.6.0

Anything else?

No response

Stusu commented 1 year ago

I think I have simpler example of the same issue: https://codepen.io/stusu/pen/rNKRmbM The issue here is that I have 2 custom fields which both calles onChange (from useEffect) at the same time, what causes that the first component does not get new value. Please check the console for the example I provided, it clearly shows that first component called onChange with new value but it was not passed to it through props.formData

heath-freenome commented 1 year ago

@Stusu @LouaiCoolshop The problem here is that the formData is stashed in a state variable on Form and the second onChange() causes the state to be updated with the original state and not the updated state. Remember react merges state in its own time, not immediately. Our code, currently does the bad thing, and not the correct thing (sorry). Do either of you feel comfortable fixing this in the 5.0.0 release?

balajiram commented 1 year ago

@heath-freenome , Before proceeding on solution recommended, I would like to confirm my assertion about the fix.

The fix is on these line in the onChange method

this.setState(
      state as FormState<T, S, F>,
      () => onChange && onChange({ ...this.state, ...state }, id)
    );

and update the logic as below

this.setState(
      (oldState) => ({
        ...oldState,
        ...state
      } as FormState<T, S, F>),
      () => onChange && onChange({ ...this.state, ...state }, id)
    );
nickgros commented 1 year ago

@balajiram @heath-freenome you may also need to modify onPropertyChange in ObjectField.tsx (link to code).

Ideally, a PR would include a test that uses a simple custom field like in @Stusu 's example to prove that the issue is fixed.

heath-freenome commented 1 year ago

@nickgros Since the only place that the state is set is in Form I'm not sure onPropertyChange() is necessary

balajiram commented 1 year ago

@heath-freenome Updating the setState with async syntax on "Form" onChange method didn't fix the change.

Reasoning? (with Example 2) useEffects starts asynchronously after the component render. So effects have old state values in their scope(state value before effects start). So the value changed one effect is not available in the scope of the other effect, so form state is not getting updated properly here.

Example 2 with updated setState changes here. check the console.log statement to assert the reasoning given.

In component updating field1 value yyy -> yyy-changed 
In component updating field2 value zzz -> zzz-changed 

Inside @rjsf/core Form onChange 
oldState field1: yyy field2: zzz 
newState field1: yyy-changed field2: zzz

Inside @rjsf/core Form onChange 
oldState field1: yyy-changed field2: zzz 
newState field1: yyy field2: zzz-changed // here field1 value should be  "yyy-changed" but its "yyy"

Inside @rjsf/core Form onChange 
oldState field1: yyy field2: zzz-changed 
newState field1: yyy-changed field2: zzz

Inside @rjsf/core Form onChange 
oldState field1: yyy-changed field2: zzz 
newState field1: yyy field2: zzz-changed

Example 1 : Updating the state inside the effects is not recommended, check here on more details. So can you update the logic and try?

heath-freenome commented 1 year ago

Ah, as it turns out the issue really is in the onPropertyChange() function that @nickgros pointed out. There is no simple fix for this. In fact, it would likely take a reimplementation of how we update formData in containers like objects and arrays. This is no simple feat as it would likely result in a major breaking change. Sorry we don't have an easy fix.

psmolaga commented 6 months ago

Hi, everyone! I have discovered that there is a flushSync API in React DOM. Perhaps this can help with synchronizing the state updates?

https://react.dev/reference/react-dom/flushSync