Search code examples
javascriptreactjsuse-effect

useEffect() causing strange behaviour when trying to update state


I'm making what I thought would be quite a simple program, which allows you to track both the amount of time men/women have spoken in a meeting, and how many times they have spoken. But there's something strange happening - it's deployed here so you can see: https://make-space.netlify.app/ If you hit the +1 increment button without the timer running, it works fine and increments perfectly. However, if you have the timer running then it goes up, then back down, then up again within about a second.

From console logging to try and work out where it's going wrong, I am pretty sure it's the useEffect() function I'm using to handle the timer, but I can't work out how to do it differently.

The thing I'm updating is "speakerCount", and that is rendered further down on the page with no problems, but it's when trying to pass it to other components that it causes an issue.

In this section:

const incrementSpeakerCount = () => {
    difference(minutes, seconds, speakerCount + 1);
    setSpeakerCount(speakerCount + 1);
  };

If I put console.log(speakerCount) it doesn't show it updating.

The "difference" prop is a function I'm passing from the parent component to get the state and pass it on.

The full code is here, but the whole thing is available on github here: https://github.com/gordonmaloney/makespace

import React, { useState, useEffect } from "react";
import { Button } from "@material-ui/core";

const Timer = ({ gender, difference }) => {
  const [seconds, setSeconds] = useState(0);
  const [minutes, setMinutes] = useState(0);
  const [isActive, setIsActive] = useState(false);
  const [speakerCount, setSpeakerCount] = useState(0);

  let speakerCountUpdated = speakerCount

  function toggle() {
    setIsActive(!isActive);
  }

  function reset() {
    difference(0, 0, 0);
    setSeconds(0);
    setMinutes(0);
    setSpeakerCount(0);
    setIsActive(false);
  }

  if (seconds === 60) {
    setSeconds(0);
    setMinutes(minutes + 1);
  }

  useEffect(() => {
    let interval = null;
    if (isActive) {
      interval = setInterval(() => {
        setSeconds((seconds) => seconds + 1);
        difference(minutes, seconds, speakerCount)
      }, 1000);
    } else if (!isActive && seconds !== 0) {
      clearInterval(interval);
    }
    return () => clearInterval(interval);
  }, [isActive, seconds]);


  const incrementSpeakerCount = () => {
    difference(minutes, seconds, speakerCount + 1);
    setSpeakerCount(speakerCount + 1);
  };

  return (
    <div>
      <center>
        <div className={"timerContainer"}>
          <div>

            <b>{gender}</b> have been speaking for:
            <br />
            {minutes} {minutes === 1 ? "minute" : "minutes"} and {seconds}{" "}
            {seconds === 1 ? "second" : "seconds"}

          </div>
          <div>
            <Button
              className={`button button-primary button-primary-${
                isActive ? "active" : "inactive"
              }`}
              variant="contained"
              color="primary"
              size="small"
              onClick={toggle}
            >
              {isActive ? "Pause" : "Start"}
            </Button>
            <br />
            <br />
            <b>
              {speakerCount === 0 ? "No" : speakerCount}{" "}
              {speakerCount !== 1
                ? gender.toLowerCase()
                : gender === "Men"
                ? "man"
                : "woman"}
            </b>{" "}
            {speakerCount !== 1 ? "have" : "has"} spoken.
            <br />
            <Button
              className="button"
              variant="contained"
              color="primary"
              size="small"
              onClick={incrementSpeakerCount}
            >
              + 1
            </Button>
            <br />
            <br />
            <Button
              className="button"
              variant="contained"
              size="small"
              onClick={reset}
            >
              Reset
            </Button>
          </div>
        </div>
      </center>
    </div>
  );
};

export default Timer;

Any ideas very welcome - I'm relatively new to lifecycle methods so suspect I may be doing something quite simple wrong.

Thanks a lot!


Solution

  • This is an interesting problem. Your Timer is called every second. But there's a missing dependency for speakerCount. Therefore in theory the speakerCount can lag behind due to setInterval.

    Imagine the time goes from 0 to 1, during that time, you clicked the button, but the speakerCount still holds the previous value in the current render. Remember if there's no render, speakerCount can't be holding a new value.

      useEffect(() => {
        let interval = null;
        if (isActive) {
          interval = setInterval(() => {
            setSeconds((seconds) => seconds + 1);
            difference(minutes, seconds, speakerCount)
          }, 1000);
        } else if (!isActive && seconds !== 0) {
          clearInterval(interval);
        }
        return () => clearInterval(interval);
      }, [isActive, seconds, speakerCount]);
    

    Try if the above works, you can't actually get exactly 1s in your case, because everytime when isActive, seconds or speakerCount changes you are trying to shoot to another second. So somehow even when it's working, it can't be exactly 1 second.