How can one prevent duplicate setTimeout resulting from complex state change?

129 views Asked by At

I apologize for the complexity of the code this question is based around, but it seems the issue itself is arising from the complexity. I wasn't able to replicate the issue with a simpler example. Here is the code repository branch with the issue: https://github.com/chingu-voyages/v42-geckos-team-21/commit/3c20cc55e66e7d0f9122d843222980e404d4910f The left hand (before the change) uses useRef() and does not have the issue, but I don't think useRef() respect's its proper usage.

Here is the main problem code:

import { useState, useRef} from 'react';
import "./Alert.css"
import "animate.css"
import { CSSTransition } from "react-transition-group"
import { time } from 'console';


console.log(typeof CSSTransition);

interface IfcProps {
  text: React.ReactNode,
  exitAfterDuration: number,
  setAlertKey: React.Dispatch<React.SetStateAction<number>>,
  alertKey: number
}

const classNames = {
  appear: 'animate__bounce',
  appearActive: 'animate__bounce',
  appearDone: 'animate__bounce',
  enter: 'animate__bounce',
  enterActive: 'animate__bounce',
  enterDone: 'animate__bounce',
  exit: 'animate__bounce',
  exitActive: 'animate__fadeOut',
  exitDone: 'animate__fadeOut'
}

function Alert(props: IfcProps) {

  const nodeRef = useRef(null);
  

  let [isIn, setIsIn] = useState(false);
  let [previousAlertKey, setPreviousAlertKey] = useState(0);
  let [timeoutId, setTimeoutId] = useState<number | null>(null);


  // console.log('props', {...props});
  console.log('prev, pres', previousAlertKey, props.alertKey)
  console.log('state', {isIn, previousAlertKey, timeoutId});

  // console.log('prev, current:', previousAlertKey, props.alertKey);
  if (props.text === '') {
    // do not render if props.text === ''
    return null;
  } else if (previousAlertKey !== props.alertKey) {
    
    setIsIn(true);
    setPreviousAlertKey(oldPreviousAlertKey => {
      
      oldPreviousAlertKey++
      return oldPreviousAlertKey;
    });


    if (timeoutId) {
      console.log(timeoutId, 'timeout cleared');
      clearTimeout(timeoutId);
    }

    let localTimeoutId = window.setTimeout(() => {
      console.log('executing timeout')
      setIsIn(false);
    }, props.exitAfterDuration);
   
    console.log({localTimeoutId}, previousAlertKey, props.alertKey);
    setTimeoutId(localTimeoutId);


  }
  
 
      


  

  return (
    <CSSTransition nodeRef={nodeRef} in={isIn} appear={true} timeout={1000} classNames={classNames}>
      {/* Using key here to trigger rebounce on alertKey change */}
      <div ref={nodeRef} id="alert" className="animate__animated animate__bounce" key={props.alertKey}>
        {props.text}
      </div>
    </CSSTransition>
  )
}

export default Alert

Code that resolves issue but probably uses useRef() incorrectly:

import { useState, useRef } from 'react';
import "./Alert.css"
import "animate.css"
import { CSSTransition } from "react-transition-group"
import { time } from 'console';


console.log(typeof CSSTransition);

interface IfcProps {
  text: React.ReactNode,
  exitAfterDuration: number,
  setAlertKey: React.Dispatch<React.SetStateAction<number>>,
  alertKey: number
}

const classNames = {
  appear: 'animate__bounce',
  appearActive: 'animate__bounce',
  appearDone: 'animate__bounce',
  enter: 'animate__bounce',
  enterActive: 'animate__bounce',
  enterDone: 'animate__bounce',
  exit: 'animate__bounce',
  exitActive: 'animate__fadeOut',
  exitDone: 'animate__fadeOut'
}

function Alert(props: IfcProps) {

  const nodeRef = useRef(null);
  const timeoutIdRef = useRef<number | null>(null);

  let [isIn, setIsIn] = useState(false);
  let [previousAlertKey, setPreviousAlertKey] = useState(0);

  console.log({props});
  console.log('state', {isIn, previousAlertKey, timeoutIdRef});

  console.log('prev, current:', previousAlertKey, props.alertKey);
  if (props.text === '') {
    // do not render if props.text === ''
    return null;
  } else if (previousAlertKey !== props.alertKey) {
    
    setIsIn(true);
    setPreviousAlertKey(oldPreviousAlertKey => {
      
      oldPreviousAlertKey++
      return oldPreviousAlertKey;
    });


    if (timeoutIdRef.current) {
      console.log(timeoutIdRef.current, 'timeout cleared');
      clearTimeout(timeoutIdRef.current);
    }

    let localTimeoutId = window.setTimeout(() => setIsIn(false), props.exitAfterDuration);
   
    console.log({localTimeoutId}, previousAlertKey, props.alertKey);
    timeoutIdRef.current = localTimeoutId;
  }
  
      


  

  return (
    <CSSTransition nodeRef={nodeRef} in={isIn} appear={true} timeout={1000} classNames={classNames}>
      {/* Using key here to trigger rebounce on alertKey change */}
      <div ref={nodeRef} id="alert" className="animate__animated animate__bounce" key={props.alertKey}>
        {props.text}
      </div>
    </CSSTransition>
  )
}

export default Alert

The issue shows its head when an invalid row is attempted to be submitted to the database and the Alert component appears. If multiple alerts are triggered in this way, they all disappear when the first setTimeout expires because it was never cleared properly. One timeout should be cleared but because React strict mode renders twice and the creation of a timeout is a side effect, the extra timeout never gets cleared. React isn't aware that there are two timeouts running for every submission attempt (check-mark click).

I'm probably handling my alert component incorrectly, with the alertKey for example.

I feel my problem is related to the fact that my setTimeout is triggered inside the Alert component as opposed to inside the Row component's onClick() handler, as I did so in a simpler example and it did not exhibit the issue.

I fear I may not get any replies as this is a pretty ugly and complex case that requires a fair bit of setup to the dev environment. This may well be a case where I just have to cobble together a solution (e.g. with useRef) and learn the proper React way in the future through experience. Tunnel-vision is one of my faults.

1

There are 1 answers

0
cyclingLinguist On BEST ANSWER

tl;dr Use the dependency array in useHook()

So I took a step back and worked on some other parts of the app, while sometimes doing some research into how others handle a toast notification component, which is what I was effectively working on in the code here. Logrocket's article was helpful: How to create a custom toast component with React.

@Azzy helped put me back on the right track and the article above also uses the useEffect() hook for the timeout.

In my spare time, eventually I came across this article A Simple Explanation of React.useEffect(). The author, Dmitri Pavlutin, finally got it into my head the intended relation of the main component function body and the useEffect hook.

A functional React component uses props and/or state to calculate the output. If the functional component makes calculations that don't target the output value, then these calculations are named side-effects.

. . .

The component rendering and side-effect logic are independent. It would be a mistake to perform side-effects directly in the body of the component, which is primarily used to compute the output.

How often the component renders isn't something you can control — if React wants to render the component, you cannot stop it.

. . .

How to decouple rendering from the side-effect? Welcome useEffect() — the hook that runs side-effects independently of rendering.

I now present the code that works and is the React way, (or at least is more like the optimal React way than my two previous attempts, see above).

function Alert(props: IfcProps) {
  useEffect(() => {
    let timeoutId = window.setTimeout(() => setIsIn(false), props.exitAfterDuration);

    return function cleanup() {
      window.clearTimeout(timeoutId);
    }

  }, [props.alertKey]);



  const nodeRef = useRef(null);
  const timeoutIdRef = useRef<number | null>(null);

  let [isIn, setIsIn] = useState(false);
  let [previousAlertKey, setPreviousAlertKey] = useState(0);

  console.log({ props });
  console.log('state', { isIn, previousAlertKey, timeoutIdRef });

  console.log('prev, current:', previousAlertKey, props.alertKey);
  if (props.text === '') {
    // do not render if props.text === ''
    return null;
  } else if (previousAlertKey !== props.alertKey) {

    setIsIn(true);
    setPreviousAlertKey(oldPreviousAlertKey => {

      oldPreviousAlertKey++
      return oldPreviousAlertKey;
    });
  }






  return (
    <CSSTransition nodeRef={nodeRef} in={isIn} appear={true} timeout={1000} classNames={classNames}>
      {/* Using key here to trigger rebounce on alertKey change */}
      <div ref={nodeRef} id="alert" className="animate__animated animate__bounce" key={props.alertKey}>
        {props.text}
      </div>
    </CSSTransition>
  )
}

export default Alert

To be honest, there's probably a lot of refactoring I can do now that I understand the utility of useEffect(). I likely have other components with side effects dependent on logic dedicated to checking if the current render happened because specific state / props changed. useEffect()'s dependency array is a lot cleaner than conditionals in the function body checking for those state/prop changes.

A lot of the complexity I lamented in my question arose I think because I wasn't properly separating side effects from my main function body.

Thank you for coming to my TED talk. :)