Using useEffect with event listeners

Question

The issue I'm having is that when I set up an event listener, the value the event listener sees doesn't update with the state. It's as if it's bound to the initial state.

What is the correct way to do this?

Simple example:

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

const App = () => {
  const [name, setName] = useState("Colin");
  const [nameFromEventHandler, setNameFromEventHandler] = useState("");

  useEffect(() => {
    document.getElementById("name").addEventListener("click", handleClick);
  }, []);

  const handleButton = () => {
    setName("Ricardo");
  };

  const handleClick = () => {
    setNameFromEventHandler(name);
  };

  return (
    <React.Fragment>
      <h1 id="name">name: {name}</h1>
      <h2>name when clicked: {nameFromEventHandler}</h2>
      <button onClick={handleButton}>change name</button>
    </React.Fragment>
  );
};

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

Gif below, since SO code snippet doesn't work for some reason.

score 7 · Accepted Answer · answered Mar 20 '19 at 14:11

7

So your problem is that you pass an empty array as the second argument to your effect so the effect will never be cleaned up and fired again. This means that handleClick will only ever be closed over the default state. You've essentially written: setNameFromEventHandler("Colin"); for the entire life of this component.

Try removing the second argument all together so the effect will be cleaned up and fired whenever the state changes. When the effect refires, the function that will be handling the click event that will be closed over the most recent version of your state. Also, return a function from your useEffect that will remove your event listener.

E.g.

  useEffect(() => {
    document.getElementById("name").addEventListener("click", handleClick);
    return () => {
      document.getElementById("name").removeEventListener("click", handleClick);
    }
  });

answered Mar 20 '19 at 14:11

Tom Finney

2,670
18
12

2

It is safe to pass `[name]` as second argument in this example – UjinT34 Mar 20 '19 at 14:13
True, that would also probably help explain what's going on a bit better - whenever the name changes, remove the old event listener and attach a new one. – Tom Finney Mar 20 '19 at 14:14
This makes sense. I also think using `[name]` is a bit nicer since in the real world I don't want to have to attach / detach listeners every time any state changes. – Colin Ricardo Mar 20 '19 at 14:18
Also this led to me fixing my realworld solution, so I'm marking it as correct. Thanks! – Colin Ricardo Mar 20 '19 at 14:18

score 3 · Answer 2 · answered Mar 20 '19 at 14:29

3

I think correct solution should be this: codesanbox. We are telling to the effect to take care about its dependency, which is the callback. Whenever it is changed we should do another binding with correct value in closure.

answered Mar 20 '19 at 14:29

Pavel Kratochvil

452
2
9

score 2 · Answer 3 · answered Jul 10 '20 at 11:28

I believe the correct solution would be something like this:

  useEffect(() => {
    document.getElementById("name").addEventListener("click", handleClick);
  }, [handleClick]);

  const handleButton = () => {
    setName("Ricardo");
  };

  const handleClick = useCallback(() => {
    setNameFromEventHandler(name)
  }, [name])

The useEffect should have handleClick as part of its dependency array otherwise it will suffer from what is known as a 'stale closure' i.e. having stale state.

To ensure the useEffect is not running on every render, move the handleClick inside a useCallback. This will return a memoized version of the callback that only changes if one of the dependencies has changed which in this case is 'name'.

Using useEffect with event listeners

3 Answers3

Linked