Search code examples
reactjsreact-hooksreact-props

React component is modifying its own props without directly changing them


I have created a React component that creates a form of n inputs and a customized SubmitButton. I'm passing data, an array property, from a parent component as initial values required to be displayed in each input field. The problem is the component has become a bit complicated and now it's modifying the props.data property - for some reason - when I set the property props.data in a local state variable inputs, then the program is changing the value of props.data whenever the value of any of the input fields changes. My goal was to keep the props.data unchanged, as it's supposed to be, then compare its values with the current changed inputs value because I wanted this form component to set the SubmitButton to disabled whenever the user tries to submit a form that has the same values as initial values.

Here comes the code:

interface Props {
    itemsNum: number
    types?: string[]
    labels?: string[]
    submitText?: string
    disableOnClick?: boolean
    enableOnInput?: boolean
    trigger?: number
    data?: any[]

    onSubmit(inputs: any[]): any
    fetchData?(): any[] | Promise<any[]>;
}

export function Form(props: Props) {
    const [inputs, setInputs] = useState<any[]>([])
    const [disabledInputs, setDisabledInputs] = useState<boolean>(false)
    const [disabledSubmit, setDisabledSubmit] = useState<boolean>(true)

    if(!equalsArr(props.data!, inputs)) {
         setInputs(props.data!)
    }

    function handleChange(e: any, i: number) {
        const { value } = e.target;

        const newArr = inputs
        newArr[i] = value
        setInputs(newArr)

        const isUndefined = hasUndefinedFields(inputs)
        if (!isUndefined)
            setDisabledSubmit(false)
        else 
            setDisabledSubmit(true)
    }

    function hasUndefinedFields(arr: any[]): boolean {
        for(const item of arr)
            if(!item)
                return true;
        return false;
    }

    function validateInputs(): boolean {
        for (const input of inputs) 
            if (!input || String(input).match(Regex.ZEROS))
                return false
        return true;
    }

    function validateArrLen(): boolean {
        if (props.itemsNum < 0)
            return false
        return true
    }

    function getInputs() {
        let inps = []
        for (let i = 0; i < props.itemsNum!; i++) {
            inps.push(
                <OneInput
                    key={i}
                    label={props.labels?.length! > 0 ? props.labels![i]! : ''}
                    type={(props.types?.length! && props.types![i]!) ? props.types![i]! : 'text'}
                    name={`input${i + 1}`}
                    onChange={(e: any) => handleChange(e, i)}
                    defaultValue={inputs[i]}
                    disabled={disabledInputs}
                />
            )
        }
        return inps
    }

    const content = validateArrLen() ?
        <>
            {getInputs()}
            {/* Submit */}
            <SubmitButton
                text={props.submitText ? props.submitText : Strings.SAVE}
                disabled={disabledSubmit}
                onSubmit={(e: any) => {
                    e.preventDefault()
                    if (validateInputs()) {
                        props.onSubmit(inputs)
                        setDisabledInputs(props.disableOnClick! || true)
                        setDisabledSubmit(props.disableOnClick! || true)
                    } else {
                        alert(Errors.NULL_FILEDS_ERROR)
                    }
                }}
            />
        </> : <div>{"Error: Conflict in lengths of properites."}</div>

    return content
}

Code of OneInput component:

import { useState } from "react"

interface OneInputProps {
    label?: string
    name: string
    type?: string
    onChange: any
    defaultValue?: string | number | undefined;
    disabled?: boolean
}

export function OneInput(props: OneInputProps) {
    const content = <label>
        {label}
        <input
            type={props.type ? props.type : 'text'}
            name={props.name}
            onChange={props.onChange}
            defaultValue={props.defaultValue}
            disabled={props.disabled}
        />
    </label>;
    return <>{content}</>
}

And I'm doing this in the parent component;

// code..
<table>
....
    return <tr key={gs.id}>
        <td>{gs.description}</td>
        <Form
            itemsNum={2}
            types={['number', 'number']}
            disableOnClick={true}
            data={[123, 233]}
            onSubmit={(inputs: any) => console.log("Submitted!"))}
         />
    </tr>
....
</table>

Solution

  • Issue

    The code is mutating the inputs state in the handleChange handler

    const [inputs, setInputs] = useState<any[]>([]);
    
    if (!equalsArr(props.data!, inputs)) {
      setInputs(props.data!); // <-- reference to props.data object
    }
    
    function handleChange(e: any, i: number) {
      const { value } = e.target;
    
      const newArr = inputs // <-- newArr is reference to inputs state
      newArr[i] = value     // <-- mutation!!
      setInputs(newArr)
    
      const isUndefined = hasUndefinedFields(inputs)
      if (!isUndefined)
        setDisabledSubmit(false)
      else 
        setDisabledSubmit(true)
    }
    

    Additionally, the if (!equalsArr(props.data!, inputs)) { ... } block is the incorrect way to initialize the inputs state to the data prop and keep it initialized. This is an unintentional side-effect.

    Solution Suggestion

    You can initialize inputs directly from props.data, and use another useEffect hook to keep it synchronized when data updates.

    const [inputs, setInputs] = useState<any[]>(props.data ?? []);
    
    useEffect(() => {
      setInputs(inputs => {
        // Check if current inputs state differs from new data
        // if they are unequal, return data as the next inputs state
        if (!equalsArr(data!, inputs)) {
          return data;
        }
        // otherwise return the current state value
        return inputs;
      });
    }, [data]);
    

    When updating React state all state, and nested state, that is being updated should be shallow copied into a new object reference, e.g. a new Javascript Array in this case.

    Consider the following refactoring suggestion:

    function handleChange(e: any, i: number) {
      const { value } = e.target;
    
      setInputs(inputs => inputs.map((arr, index) =>
        index === i
          ? value
          : err
      ));
    
      const isUndefined = hasUndefinedFields(inputs);
      setDisabledSubmit(isUndefined);
    }
    

    Key Points:

    1. Uses a functional state update to access the current inputs state value
    2. Uses Array.map to create a new array reference
    3. Uses the mapping index to update the correct array element with the new value

    Additional Note

    While not a contributor to the props mutation issue you asked about I feel I must also mention an oddity with your form elements, which appear to be "semi-controlled". Fully controlled inputs use React state and onChange and value props, and fully uncontrolled inputs use the defaultValue prop. Your code uses the defaultValue prop yet passes an onChange handler and maintains local state. My suggestion would be to convert to fully controlled inputs.

    OneInput

    interface OneInputProps {
      label?: string
      name: string
      type?: string
      onChange: any
      value: string | number;
      disabled?: boolean
    }
    
    export function OneInput(props: OneInputProps) {
      return (
        <label>
          {props.label}
          <input
            type={props.type || 'text'}
            name={props.name}
            onChange={props.onChange}
            value={props.value}
            disabled={props.disabled}
          />
        </label>
      );
    }
    

    Form

    function getInputs() {
      const inps = [];
    
      for (let i = 0; i < props.itemsNum!; i++) {
        inps.push(
          <OneInput
            key={i}
            label={props.labels?.length! > 0 ? props.labels![i]! : ''}
            type={(props.types?.length! && props.types![i]!) ? props.types![i]! : 'text'}
            name={`input${i + 1}`}
            onChange={(e: any) => handleChange(e, i)}
            value={inputs[i] ?? ""}
            disabled={disabledInputs}
          />
        );
      }
      return inps;
    }