2

Why my interval is speeding up? When I press any of my buttons NextImage() or PrevImage() my interval starts speeding up and the image starts glitching. Any advice or help? Here's my code =>

//Image is displayed
  const [image, setImage] = React.useState(1);
  let imageShowed;
  if (image === 1) {
    imageShowed = image1;
  } else if (image === 2) {
    imageShowed = image2;
  } else if (image === 3) {
    imageShowed = image3;
  } else {
    imageShowed = image4;
  }

  // Auto change slide interval
  let interval = setInterval(
    () => (image === 4 ? setImage(1) : setImage(image + 1)),
    5000
  );
  setTimeout(() => {
    clearInterval(interval);
  }, 5000);

  // Change image functionality
  const ChangeImage = (index) => {
    setImage(index);
  };
  / /Next image
  const NextImage = () => {
    image === 4 ? setImage(1) : setImage(image + 1);
  };

  // Previous image
  const PrevImage = () => {
    image === 1 ? setImage(4) : setImage(image - 1);
  };
Arvis Iljins
  • 335
  • 3
  • 10
  • 3
    OMG bro what is this, you shouldn't use your logic inside the component like this! wrap them inside the function and use react lifecycle like `useEffect` for rerender your component!! – b3hr4d Jan 23 '21 at 07:51
  • add sample code in codesandbox for debugging is better – A.R.SEIF Jan 23 '21 at 07:51

4 Answers4

4

When you need to have some logic which is depend on changing a variable, it's better to keep those logic inside useEffect

const interval = useRef(null);
const timeout = useRef(null);
useEffect(() => {
  interval.current = setInterval(
    () => (image === 4 ? setImage(1) : setImage((i) => i + 1)),
    5000
  );
  timeout.current = setTimeout(() => {
    clearInterval(interval.current);
  }, 5000);

  return () => {
    clearInterval(interval.current);
    clearTimeout(timeout.current);
  }
}, [image]);

one point to remember is that if you use a variable instead of using useRef it can increase the possibility of clearing the wrong instance of interval or timeout during the rerenders. useRef can keep the instance and avoid any unwanted bugs

  • const interval can be placed inside useEffect without useRef. – Kirill Skomarovskiy Jan 23 '21 at 07:59
  • @KirillSkomarovskiy no it can increase the possibility of clearing the wrong `instance` of `interval or timeout` during the rerenders. `useRef` can keep the `instance` –  Jan 23 '21 at 08:09
  • Check again your variable is local for useEffect. And never used outside useEffect so it can be used without useRef. If you have two useEffects that use interval you should use useRef. But in this example - no. – Kirill Skomarovskiy Jan 23 '21 at 08:22
  • @KirillSkomarovskiy Check this answer Kirill, you would understand my point. https://stackoverflow.com/a/65831939/15045569 –  Jan 23 '21 at 08:31
  • @ Weblandtk I've create sandbox: https://codesandbox.io/s/nifty-keller-72whk. First table is your solution with ref. Second table is solution without ref. And I add third table where I would show how make the same behavior but not clean up timerId and interval Id every value changes. Note that solution with ref and without ref create and clean timerId and intervalId too many times. – Kirill Skomarovskiy Jan 23 '21 at 15:40
2

Your approach causes so many problems and you should learn more about react (watch youtube tutorials about react), I did make a working example slider hope to help you and people in the future:

let interval;
const images = [
  "https://picsum.photos/300/200?random=1",
  "https://picsum.photos/300/200?random=2",
  "https://picsum.photos/300/200?random=3",
  "https://picsum.photos/300/200?random=4",
  "https://picsum.photos/300/200?random=5",
];
const App = () => {
  const [slide, setSlide] = React.useState(0);
  React.useEffect(() => {
    interval = setInterval(() => {
      NextSlide();
      clearInterval(interval);
    }, 5000);
    return () => {
      clearInterval(interval);
    };
  }, [slide]);

  const ChangeSlideDots = (index) => {
    setSlide(index);
  };
  const NextSlide = () =>
    setSlide((prev) => (slide === images.length - 1 ? 0 : prev + 1));

  const PrevSlide = () =>
    setSlide((prev) => (slide === 0 ? images.length - 1 : prev - 1));

  return (
    <div style={styles.root}>
      <img style={styles.imageDiv} src={images[slide]} />
      <button style={styles.buttons} onClick={PrevSlide}>
        ◁
      </button>
      <div style={styles.dotDiv}>
        {images.map((_, i) => (
          <div
            key={i}
            style={i === slide ? styles.redDot : styles.blackDot}
            onClick={() => ChangeSlideDots(i)}
          >
            .
          </div>
        ))}
      </div>
      <button style={styles.buttons} onClick={NextSlide}>
        ▷
      </button>
    </div>
  );
}
const styles = {
  root: {
    display: "flex",
    position: "relative",
    width: 300,
    height: 200,
  },
  buttons: {
    backgroundColor: "rgb(255 255 255 / 37%)",
    border: "none",
    zIndex: 2,
    flex: 1,
  },
  imageDiv: {
    position: "absolute",
    zIndex: 1,
    width: 300,
    height: 200,
  },
  dotDiv: {
    flex: 10,
    zIndex: 2,
    fontSize: "30px",
    display: "flex",
    justifyContent: "center",
  },
  redDot: {
    cursor: "pointer",
    color: "red",
  },
  blackDot: {
    cursor: "pointer",
    color: "black",
  },
};

ReactDOM.render(<App />, document.getElementById("react"));
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.4/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.4/umd/react-dom.production.min.js"></script>
<div id="react"></div>
b3hr4d
  • 4,012
  • 1
  • 11
  • 24
1

Anytime that you rerender your component, you will run the whole function once. So you will set an interval every time you use setImage(). In order to prevent this, you have to use side effect functions. here you should use useEffect() because you have a functional component. in order to make useEffect() only run once, you have to pass an empty array for dependecy array; So your useEffect will act like componentDidMount() in class components. try the code below:

let interval = null
    useEffect(() => {
        interval = setInterval(
            () => (image === 4 ? setImage(1) : setImage(image + 1)),
            5000
        )
        setTimeout(() => {
            clearInterval(interval);
        }, 5000)
    }, [])
Ardalan
  • 723
  • 9
  • 26
0

Thanks, everybody for your great answers appreciated a lot your time and help! So, my final solution looks like this:

const images = [image1, image2, image3, image4];
const quotes = [
  'Jobs fill your pockets, adventures fill your soul',
  'Travel is the only thing you buy that makes you richer',
  'Work, Travel, Save, Repeat',
  'Once a year, go someplace you’ve never been before',
];
const App = () => {
  //Image is displayed
  const [image, setImage] = React.useState(0);

  // Auto change slide interval
  useEffect(() => {
    return () => {
      clearInterval(
        setInterval((interval) => {
          image === 3 ? setImage(1) : setImage(image + 1);
          clearInterval(interval.current);
        }, 5000)
      );
    };
  }, [image]);

  // Change image functionality
  const ChangeImage = (index) => {
    setImage(index);
  };

  //Next image
  const NextImage = () => {
    image === 3 ? setImage(1) : setImage(image + 1);
  };

  // Previous image
  const PrevImage = () => {
    image === 1 ? setImage(3) : setImage(image - 1);
  };

  return (
    <Section>
      <div className='slideshow-container'>
        <div>
          <img className='slider_image' src={images[image]} alt='slider' />
          <h1 className='slider_title'>{quotes[image]}</h1>
        </div>
        <button className='slider_prev' onClick={PrevImage}>
          &#10094;
        </button>
        <button className='slider_next' onClick={NextImage}>
          &#10095;
        </button>
      </div>
      <div>
        <div>
          {images.map((image, i) => (
            <img
              key={i}
              alt={`slider${i}`}
              src={image}
              className='bottom_image'
              onClick={() => ChangeImage(i)}
            ></img>
          ))}
        </div>
      </div>
    </Section>
  );
};
Arvis Iljins
  • 335
  • 3
  • 10