CheckBox state doesn't update-CodePudding

I'm writing an application to take orders in a restaurant and on adding items, you can check some options with simple checkboxes. Howerver, when using a state for this checkbox (to capture the change and to update my optionList), the checkbox doesn't get toggled. I have the following code in my react application:

import {useState} from "react";

const Option = ({option, optionList, setOptionList}) => {
    const [checked, setChecked] = useState<boolean>(option.initialState);

    const changeOptions = () => {
        setChecked(!checked);
        if (checked) {
            setOptionList([...optionList]   option);
        } else {
            setOptionList([...optionList].filter(o => o.optionId != option.optionId));
        }
    }

    return (
        <div className={"option"}>
            <form>
                <input type="checkbox" id="option" checked={checked} onChange={changeOptions}/>
                <label htmlFor="option">{option.description}</label>
            </form>
        </div>
    );
};

export default Option;

If I change the onChange function to

() => setChecked(!checked)

the checkbox works again but I really need an update of the optionList. What am I missing?

CodePudding user response：

I would let the parent component handle the optionList as follows:

Code sandbox

import { useState } from "react";

const descriptions = ["A", "B", "C"];

interface OptionProps {
  description: string;
  checked: boolean;
  onCheck: (checked: boolean) => void;
}

const Option = ({ description, checked, onCheck }: OptionProps) => {
  return (
    <>
      <input
        type="checkbox"
        id={description}
        checked={checked}
        onChange={() => onCheck(!checked)}
      />
      <label htmlFor={description}>{description}</label>
    </>
  );
};

const Parent = () => {
  const [optionList, setOptionList] = useState<string[]>([]);
  return (
    <form>
      {descriptions.map((d) => (
        <Option
          description={d}
          checked={optionList.indexOf(d) >= 0}
          onCheck={(checked) =>
            setOptionList((prev) =>
              checked ? [...prev, d] : prev.filter((i) => i !== d)
            )
          }
        />
      ))}

      <div>{optionList.join(",")}</div>
    </form>
  );
};

Note

I moved the form to the parent
I gave each input its own ID and made each label point to that ID, since otherwise clicking the label would cause issues
The input for the option has been simplified to only the description.
I used Functional updates to update the optionList (setOptionList((prev) => ...)), because I feel it's a much safer approach than relying on the last rendered value being the previous value (though in this case, it will not matter). Its good practice, especially when you start using useState in other hooks (e.g. useEffect, useCallback, ...).

Motivation

Option shouldn't be aware of where it's being used. It's better to keep components like this as dumb as possible and let it's parent handle its behavior.
The advantage of this approach is that if the logic were to change, you'd only need to change one component (instead of both).

This is the Single-responsibility principle, which is referenced in Thinking in React