why this function is behaving weirdly?

Question

I've a state named modal in my React App. The initial value is an object that says {show: false, photo: null}.

I've two buttons in the page. One is calling the close function and another is calling the open function. open is setting the state to {show: true, photo: true} and close is just logging modal

I also wrote some code to call the close function when the Esc button is clicked.

Here's my code:

function App() {
  const [modal, setModal] = useState({ show: false, photo: null });

  // open func
  function open() {
    setModal({ show: true, photo: true });
  }

  // close func
  function close() {
    console.log(modal);
  }

  // function for esc key press
  function escFunc(event) {
    if (event.key === `Escape`) {
      close();
    }
  }
  useEffect(() => {
    document.addEventListener(`keydown`, escFunc, true);

    return () => {
      document.removeEventListener(`keydown`, escFunc, true);
    };
  }, []);

  return (
    <>
      <button onClick={open}>open</button>
      <br />
      <button onClick={close}>close</button>
    </>
  );
}

so now when I click the open button and then click the close button, it's logging {show: true, photo: true} (as expected). but the problem comes in if I press Esc now. It should log {show: true, photo: true} (as the state is already updated by the open function), but it's logging {show: false, photo: null} as if the state hasn't changed yet

Why is it happening?

Does this answer your question? [Why would a value get stale using closure in React?](https://stackoverflow.com/questions/72075881/why-would-a-value-get-stale-using-closure-in-react) — Konrad, Jan 06 '23 at 17:26
@Konrad can you please tell me why you suggested wrapping `escFunc` with `useCallback()` in Giorgi's answer? will it make any important changes? as far I know, `useCallback()` should be used when a function is causing unnecessary re-renders in your component and you wanna cache that function. And `useCallback()` should only be considered as a performance optimization — Pratik Dev, Jan 06 '23 at 17:55
[docs](https://beta.reactjs.org/reference/react/useCallback): *useCallback is a React Hook that lets you cache a function definition between re-renders.* -`useCallback` is usually used if you want one function reference in each render. Your `useEffect` is calling `escFunc` so you should add this function to the dependency array, but it would cause calling `useEffect` in each render, so `useCallback` would prevent that — Konrad, Jan 06 '23 at 17:58
i can just add `escFunc` in the dep array of `useEffect()` and it'll do the job I guess. why would I need to cache it? — Pratik Dev, Jan 06 '23 at 18:01
@PratikDev my previous suggestion doesn't make sense. What about moving `escFunc` into the useEffect, and add `modal` to the dependency array? — evolutionxbox, Jan 06 '23 at 18:04

score 0 · Answer 1 · answered Jan 06 '23 at 18:08

Whenever a component rerenders, the entire function is reran.

In your useEffect, which is only called on the first render, you call document.addEventListener with the callback function escFunc. This escFunc has a closure that stores the value of modal, which is a reference to the original object state { show: false, photo: null }.

In your open function, you set the state to { show: true, photo: true } using the object literal syntax, which creates a whole new object with a new reference location.

The event listener is still tracking the original object.

To be able to get the new state reference, you need to remove the old event listener and then add a new event listener.

There are multiple ways to do this.

useEffect(() => {
  document.addEventListener(`keydown`, escFunc, true);
  return () => {
    document.removeEventListener(`keydown`, escFunc, true);
  };
}, [modal]); // add modal to dep array

useEffect(() => {
  document.addEventListener(`keydown`, escFunc, true);
  return () => {
    document.removeEventListener(`keydown`, escFunc, true);
  };
}, [escFunc]); // add escFunc to dep array, but this would run every render

Stylistically, this is the best option because it properly shows dependencies and doesn't have extra rerenders, but the calls to useCallback might make it slower

const close = useCallback(function() {
  console.log(modal);
}, [modal]); // depends on modal

const escFunc = useCallback(function(event) {
  if (event.key === `Escape`) {
     close();
  }
}, [close]); // depends on close

useEffect(() => {
  document.addEventListener(`keydown`, escFunc, true);
  return () => {
    document.removeEventListener(`keydown`, escFunc, true);
  };
}, [escFunc]); // add escFunc to dep array

In fact, you don't even need to have escFunc outside of useEffect if you don't use it elsewhere

const close = useCallback(function() {
  console.log(modal);
}, [modal]); // depends on modal

const escFunc = useCallback(function(event) {
  if (event.key === `Escape`) {
     close();
  }
}, [close]); // depends on close

useEffect(() => {
  function escFunc(event) {
    if (event.key === `Escape`) {
      close();
    }
  }

  document.addEventListener(`keydown`, escFunc, true);
  return () => {
    document.removeEventListener(`keydown`, escFunc, true);
  };
}, [close]); // add escFunc to dep array

I'm actually confused. are you suggesting to use `useCallback()` or not? — Pratik Dev, Jan 06 '23 at 18:17
You can use `useCallback` to reduce the number of rerenders, but if your renders are cheap (like they are in the example) you don't need to worry about it — Samathingamajig, Jan 06 '23 at 18:23

why this function is behaving weirdly?

1 Answers1