5

I've been trying to build an React app with multiple alerts that disappear after a set amount of time. Sample: https://codesandbox.io/s/multiple-alert-countdown-294lc

import React, { useState, useEffect } from "react";
import ReactDOM from "react-dom";

import "./styles.css";

function TimeoutAlert({ id, message, deleteAlert }) {
  const onClick = () => deleteAlert(id);
  useEffect(() => {
    const timer = setTimeout(onClick, 2000);
    return () => clearTimeout(timer);
  });
  return (
    <p>
      <button onClick={onClick}>
        {message} {id}
      </button>
    </p>
  );
}
let _ID = 0;
function App() {
  const [alerts, setAlerts] = useState([]);
  const addAlert = message => setAlerts([...alerts, { id: _ID++, message }]);
  const deleteAlert = id => setAlerts(alerts.filter(m => m.id !== id));
  console.log({ alerts });
  return (
    <div className="App">
      <button onClick={() => addAlert("test ")}>Add Alertz</button>
      <br />
      {alerts.map(m => (
        <TimeoutAlert key={m.id} {...m} deleteAlert={deleteAlert} />
      ))}
    </div>
  );
}

const rootElement = document.getElementById("root");
ReactDOM.render(<App />, rootElement);

The problem is if I create multiple alerts, it disappears in the incorrect order. For example, test 0, test 1, test 2 should disappear starting with test 0, test 1, etc but instead test 1 disappears first and test 0 disappears last.

I keep seeing references to useRefs but my implementations don't resolve this bug.


With @ehab's input, I believe I was able to head down the right direction. I received further warnings in my code about adding dependencies but the additional dependencies would cause my code to act buggy. Eventually I figured out how to use refs. I converted it into a custom hook.

function useTimeout(callback, ms) {
  const savedCallBack = useRef();
  // Remember the latest callback
  useEffect(() => {
    savedCallBack.current = callback;
  }, [callback]);
  // Set up timeout
  useEffect(() => {
    if (ms !== 0) {
      const timer = setTimeout(savedCallBack.current, ms);
      return () => clearTimeout(timer);
    }
  }, [ms]);
}
SILENT
  • 3,916
  • 3
  • 38
  • 57

3 Answers3

3

You have two things wrong with your code,

1) the way you use effect means that this function will get called each time the component is rendered, however obviously depending on your use case, you want this function to be called once, so change it to

 useEffect(() => {
    const timer = setTimeout(onClick, 2000);
    return () => clearTimeout(timer);
  }, []);

adding the empty array as a second parameter, means that your effect does not depend on any parameter, and so it should only be called once.

Your delete alert depends on the value that was captured when the function was created, this is problematic since at that time, you don't have all the alerts in the array, change it to

const deleteAlert =  id => setAlerts(alerts => alerts.filter(m => m.id !== id));

here is your sample working after i forked it https://codesandbox.io/s/multiple-alert-countdown-02c2h

ehab
  • 7,162
  • 1
  • 25
  • 30
  • I never knew setAlerts can be used like a function. How did you learn that? All the documentation I read show its used something like `setAlerts([])`. – SILENT Jun 26 '19 at 19:27
  • 1
    @SILENT you can find that in the hooks api reference https://reactjs.org/docs/hooks-reference.html. but you can also assume that it will be there, setState hook should have a way to update the state based on prvState, just like the setState method in class components. and a first thought is that they would be the same regarding that manner - though they are similar there is actually some differences between the two, one of the them is that the setState hook does not merge the state like the setState class method – ehab Jun 26 '19 at 19:37
1

well your problem is you remount on every re-render, so basically u reset your timers for all components at time of rendering.

just to make it clear try adding {Date.now()} inside your Alert components

      <button onClick={onClick}>
        {message} {id} {Date.now()}
      </button>

you will notice the reset everytime

so to achieve this in functional components you need to use React.memo

example to make your code work i would do:

const TimeoutAlert = React.memo( ({ id, message, deleteAlert }) => {
  const onClick = () => deleteAlert(id);
  useEffect(() => {
    const timer = setTimeout(onClick, 2000);
    return () => clearTimeout(timer);
  });
  return (
    <p>
      <button onClick={onClick}>
        {message} {id}
      </button>
    </p>
  );
},(oldProps, newProps)=>oldProps.id === newProps.id) // memoization condition

2nd fix your useEffect to not run cleanup function on every render

useEffect(() => {
  document.title = `You clicked ${count} times`;
}, [count]); // Only re-run the effect if count changes

finally something that is about taste, but really do you need to destruct the {...m} object ? i would pass it as a proper prop to avoid creating new object every time !

Zalaboza
  • 8,899
  • 16
  • 77
  • 142
1

Both answers kind of miss a few points with the question, so after a little while of frustration figuring this out, this is the approach I came to:

  • Have a hook that manages an array of "alerts"
  • Each "Alert" component manages its own destruction

However, because the functions change with every render, timers will get reset each prop change, which is undesirable to say the least.

It also adds another lay of complexity if you're trying to respect eslint exhaustive deps rule, which you should because otherwise you'll have issues with state responsiveness. Other piece of advice, if you are going down the route of using "useCallback", you are looking in the wrong place.

In my case I'm using "Overlays" that time out, but you can imagine them as alerts etc.

Typescript:

// useOverlayManager.tsx
export default () => {
  const [overlays, setOverlays] = useState<IOverlay[]>([]);

  const addOverlay = (overlay: IOverlay) => setOverlays([...overlays, overlay]);
  const deleteOverlay = (id: number) =>
    setOverlays(overlays.filter((m) => m.id !== id));

  return { overlays, addOverlay, deleteOverlay };
};

// OverlayIItem.tsx
interface IOverlayItem {
  overlay: IOverlay;
  deleteOverlay(id: number): void;
}

export default (props: IOverlayItem) => {
  const { deleteOverlay, overlay } = props;
  const { id } = overlay;

  const [alive, setAlive] = useState(true);

  useEffect(() => {
    const timer = setTimeout(() => setAlive(false), 2000);
    return () => {
      clearTimeout(timer);
    };
  }, []);

  useEffect(() => {
    if (!alive) {
      deleteOverlay(id);
    }
  }, [alive, deleteOverlay, id]);

  return <Text>{id}</Text>;
};

Then where the components are rendered:

  const { addOverlay, deleteOverlay, overlays } = useOverlayManger();
  const [overlayInd, setOverlayInd] = useState(0);

  const addOverlayTest = () => {
    addOverlay({ id: overlayInd});
    setOverlayInd(overlayInd + 1);
  };


  return {overlays.map((overlay) => (
            <OverlayItem
              deleteOverlay={deleteOverlay}
              overlay={overlay}
              key={overlay.id}
            />
          ))};

Basically: Each "overlay" has a unique ID. Each "overlay" component manages its own destruction, the overlay communicates back to the overlayManger via prop function, and then eslint exhaustive-deps is kept happy by setting an "alive" state property in the overlay component that, when changed to false, will call for its own destruction.

user37309
  • 597
  • 5
  • 14
  • Hello, I am trying to use this code but when i use it says IOverlay is missing..? – infamous hvher Feb 02 '22 at 21:37
  • @infamoushvher move "interface IOverlayItem" part to a different file, and import it in both pieces of code :) – user37309 Feb 03 '22 at 07:29
  • Hello, I notice there is some bug with your code. When spamming notifications it will start to bug out and act weirdly. I belive this is due to the deleteNotification in the hook itself. And it need's a useEffect. @user37309 – infamous hvher Feb 28 '22 at 20:22
  • You could try putting addOverlayTest in a useCallback, and change setOverlayInd(overlayInd + 1) to setOverlayInd(i => i + 1); to be sure? – user37309 Mar 01 '22 at 03:59
  • Can you show me more in depth, I am really trying to make this work as flawlessly as possible! – infamous hvher Mar 02 '22 at 00:35
  • This solution worked for me. It complies with exhaustive-deps (in useEffect) and it doesn't resort to React.memo, which is not to be encouraged as that is to be used for performance optimization only. Be careful that you specify the deps correctly in the useEffect blocks and it will work. – ghostypants Jul 06 '23 at 07:51