CODEWITHSUNDEEP
javascriptreactjsuse-effectuse-state
Fransedo Ravelino
Fransedo Ravelino

Reputation: 33

Use Effect Triggered Multiple Times

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!

Upvotes: 0

Views: 889

Answers (4)

Bart Krakowski
Bart Krakowski

Reputation: 1670

You need to clean up the events:

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);

    return () => window.removeEventListener("click", changePower) <-----
  }, [power])

In addition: the changePower function should be declared outside of the useEffect hook. You use the useCallback hook here.

Upvotes: 0

Codshark11
Codshark11

Reputation: 87

If you set Power in the useEffect, it will trigger itself.

useEffect(() => {
   if(count%2)
      document.getElementById('btn-trigger').style.backgroundColor = "red";
   else
      document.getElementById('btn-trigger').style.backgroundColor = "lime";
   }, [count])

const handeClick = () => {
  setCount(count + 1);
}

Upvotes: 1

T.J. Crowder
T.J. Crowder

Reputation: 1075239

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]);

Upvotes: 0

Eden Zadikove
Eden Zadikove

Reputation: 24

you are setting "power" value by "setPower" inside a useEffect who is listening to changes on "power".

Upvotes: 0

Related Questions