Use Effect Triggered Multiple Times

Question

I'm trying to do a simple setState on a button with a counter and apply different background color depending on its state. It runs perfectly for the first 3 button clicks and on the fourth one and so on it does this: counter log

Here's the code:

useEffect(() => {
    const changePower = () => {
      if (power === 'on') {
        document.getElementById('btn-trigger').style.backgroundColor = "red";
        setPower('off');
      } else if (power==='off') {
        document.getElementById('btn-trigger').style.backgroundColor = "lime";
        setPower('on');
      }
      setCount(count + 1);
    }

    document.getElementById('btn-trigger').addEventListener('click', changePower);
    console.log(count);
  }, [power])

Any help would be awesome, Thank You!

Answer 1

You're adding a new event listener every time you change power, without removing the previous one. So the first click calls one handler, the second calls two, the third calls three, etc.

Using addEventListener in a React component is usually (though not always) an anti-pattern. Instead, hook up the handler via React props. Similarly, you'd change the background color the same way. That also does away with the need for the useEffect:

const {useState} = React;

const Example = () => {
    const [power, setPower] = useState("off"); // Note: I'd use a boolean, not strings

    const changePower = () => {
        setPower(p => p === "off" ? "on" : "off");
    };

    return <span className={`power ${power}`} onClick={changePower}>power</span>;
};

ReactDOM.render(<Example/>, document.getElementById("root"));

.power {
    cursor: pointer;
}
.on {
    background-color: lime;
    color: black;
}
.off {
    background-color: red;
    color: white;
}

<div id="root"></div>

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/17.0.2/umd/react.development.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/17.0.2/umd/react-dom.development.js"></script>

But if you really need to use an external element, return a cleanup callback from useEffect to remove the previous event handler. I'd probably also look up the element in one place rather than multiple:

useEffect(() => {
    const trigger = document.getElementById('btn-trigger'); // ***
    const changePower = () => {
        if (power === 'on') {
            trigger.style.backgroundColor = "red";
            setPower('off');
        } else if (power==='off') {
            trigger.style.backgroundColor = "lime";
            setPower('on');
        }
        setCount(count + 1);
    }

    trigger.addEventListener('click', changePower);
    console.log(count);
    // ***vvv
    return () => {
        trigger.removeEventListener('click', changePower);
    };
    // ***^^^
}, [power]);