2

I am trying to build a timer with three buttons, a start, stop, where it stops on the current integer, and a reset. I have added my code below which results in the following issues.

I thought my stop function would stop the timer from decrementing but it continues to do so. Also, when logging my timer state in the console, you can see it does not update in the console even though it is updating in the DOM. Why is this?

Thank you for any insight at all.

import React from 'react';
import './style.css';

export default function App() {
  const [timer, setTimer] = React.useState(50);

  const reset = () => {
    setTimer(50);
  };

  const start = () => {
    setTimer((prev) => prev - 1);
  };
  // const interval = setInterval(() => {
  //   console.log(updated)
  //   //start() }, 1000)
  //  }

  const interval = () => {
    setInterval(() => {
      console.log('updated');
      console.log(timer);
      start();
    }, 1000);
  };

  const stop = () => {
    clearInterval(start);
  };

  return (
    <div>
      <h1>{timer}</h1>
      <button onClick={interval}>start</button>
      <button onClick={stop}>stop</button>
      <button onClick={reset}>reset</button>
    </div>
  );
}`

MouseWarrior
  • 391
  • 4
  • 19
  • 1
    This is a little confusing. `clearTimeout` and `clearInterval` expect timer/interval references. You're passing a function to `clearInterval` and nothing at all to `clearTimeout`. It's also not clear why you're using both; while you *can* (refs are shared between intervals and timers) it's confusing. – Dave Newton May 12 '22 at 17:15
  • The clearTimeout was old, didnt mean to inlcude it. Updated the post – MouseWarrior May 12 '22 at 17:16
  • 1
    Rest of comments still stand. If you don't save the `interval` ref you can't stop it. – Dave Newton May 12 '22 at 17:18
  • Not sure how to go about that... Should I use clearInterval and create a ref for it? I am not sure how to create a ref for it and then reference it. – MouseWarrior May 12 '22 at 17:20
  • You *are* using `clearInterval`, but you're passing in a function. https://developer.mozilla.org/en-US/docs/Web/API/setInterval#return_value – Dave Newton May 12 '22 at 17:43

3 Answers3

3

You have a small problem with assigning the actual value for the interval.

Here is how it should be in usage

const interval = setInterval(() => {})

clearInterval(interval)

For your code change, you can create a ref to keep the interval variable and use it to clean up the interval later.

function App() {
  const [timer, setTimer] = React.useState(5);
  const intervalRef = React.useRef(); //create a ref for interval

  const reset = () => {
    setTimer(5);
  };

  const start = () => {
    setTimer((prev) => {
       if(prev === 0) {
          stop();
          return 0;
       }
       return prev - 1;
    });
  };
  // const interval = setInterval(() => {
  //   console.log(updated)
  //   //start() }, 1000)
  //  }

  const interval = () => {
    //assign interval ref here
    intervalRef.current = setInterval(() => {
      start();
    }, 1000);
  };

  const stop = () => {
    //clear the interval ref
    clearInterval(intervalRef.current);
  };

  return (
    <div>
      <h1>{timer}</h1>
      <button onClick={interval}>start</button>
      <button onClick={stop}>stop</button>
      <button onClick={reset}>reset</button>
    </div>
  );
}

ReactDOM.render(
  <App/>,
  document.getElementById("root")
);
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.0/umd/react-dom.production.min.js"></script>
<div id="root"></div>
Nick Vu
  • 14,512
  • 4
  • 21
  • 31
  • Thank you for your time Nick! If I wanted the timer to stop when it reaches 0, how could I go about that? I cant compare timer to 0 because it is always 50 and using the intervalRef results in weird functionality. – MouseWarrior May 12 '22 at 18:12
  • 1
    I just added that logic in `interval` too. You can check it out @guts716 – Nick Vu May 12 '22 at 18:15
  • Did you test that by chance? It doesnt seem to work for me. – MouseWarrior May 12 '22 at 18:17
  • 1
    I added runnable code for your case and modified my logic as well, you can verify it again @guts716 – Nick Vu May 12 '22 at 18:23
  • 1
    Be careful with the callback you assign to `setInterval`. If you access any state inside that callback, it will be stale and give you unintuitive behavior because the callback's captured variables won't update on rerenders. For instance, adding a `console.log(timer)` right before `start()` will continuously print `5`. The same would happen with any other state or props. – MHebes May 12 '22 at 18:46
1

clearTimeout or clearInterval each take a token, which was returned by a previous call to setTimeout or setInterval. So you'll need to store that token:

const id = setInterval(() => console.log("triggered"), 1000);
// ...
clearInterval(id)

Also, you should be careful about what happens if App is re-rendered, so you should probably put the set/clear logic inside useEffect so you can cleanup your interval.

Also also, although you didn't ask, your console.log(timer) isn't going to work, and will always print 50. The timer variable inside that callback is captured once, and is never updated because that callback is just inside the setInterval now. You'll need to clear and reset your interval with an updated callback function every time App re-renders, or use a ref that you keep updated, which is a pain.

I would recommend borrowing this custom hook that considers all of these things for you: https://usehooks-ts.com/react-hook/use-interval

Then your App component could become extremely simple, but still be robust:

const { useEffect, useRef, useState, useLayoutEffect } = React;

// https://usehooks-ts.com/react-hook/use-interval
function useInterval(callback: () => void, delay: number | null) {
  const savedCallback = useRef(callback);

  useLayoutEffect(() => {
    savedCallback.current = callback;
  }, [callback])

  useEffect(() => {
    if (!delay && delay !== 0) return;
    const id = setInterval(() => savedCallback.current(), delay);
    return () => clearInterval(id);
  }, [delay]);
}

function App() {
  const [timer, setTimer] = useState(50);
  const [running, setRunning] = useState(false);
  
  useInterval(() => setTimer(t => t - 1), running ? 1000 : null);
  
  const start = () => setRunning(true);
  const stop = () => setRunning(false);
  const reset = () => { setTimer(50); };

  return (
    <div>
      <h1>{timer}</h1><button onClick={start}>start</button>
      <button onClick={stop}> stop </button>
      <button onClick={reset}> reset </button>
    </div>
  );
}

ReactDOM.render(<App />, document.getElementById("react"));
<div id="react"></div>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/17.0.1/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/17.0.1/umd/react-dom.production.min.js"></script>
MHebes
  • 2,290
  • 1
  • 16
  • 29
  • Thank you for your time! Do you see anything wrong performance wise with the answer I awarded? Nick Vu's answer seems more intuitive at first glance. – MouseWarrior May 12 '22 at 18:08
  • 1
    @guts716 Performance-wise no, but it may give you strange behavior if you’re not careful. See my comment on that answer. This custom hook avoids that problem and correctly re-assigns the callback on re-renders. – MHebes May 12 '22 at 18:47
-1

I think you should not wrap your setTimeout into a callable. Then you lose the ability to start and stop it because the variable does not reference the interval but the callable that wraps the interval.

Take a look at this guide: https://www.w3schools.com/jsref/met_win_clearinterval.asp

Jnt0r
  • 174
  • 3
  • 16