5

Please take a look at this code snippet.

function App() {
  useEffect(() => {
    document.addEventListener('keydown', onKeyDown);
  }, []);

  const onKeyDown = e => {
    setKeyMap({ ...keyMap, [e.keyCode]: true });
  };

  const [keyMap, setKeyMap] = useState({});

  // JSON.stringify gives only one key-value pair
  return (
    <div className="App">
      <h1>Hello {JSON.stringify(keyMap)}</h1>
    </div>
  );
}

So using useEffect, I am adding an eventlistener to keydown event only once. This is similar to componentDidMount apparently since this occurs only once.

In the event handler for keydown event, I am trying to add all keycodes which were pressed to be true.

So say I press 'a', then my keyMap should look like this:

{
    "65": true
}

Now if I press 'b', then my keyMap should look like this:

{
    "65": true,
    "66": true
}

But what's happening is this:

{
    "66": true
}

Why am I unable to mutate the previous state and add to it? Like I previously had "65" as the key, but when I press another key on my keyboard, suddenly this "65" key-value disappears. That is at any given point, only one key-value pair exists for some reason. I want to be able to add to this object(keyMap object I mean) as many times as I want. Is the ... operator(spread) not working?

theprogrammer
  • 1,698
  • 7
  • 28
  • 48

2 Answers2

5
useEffect(() => {
    document.addEventListener('keydown', onKeyDown);
}, []);

Due to the empty array as the second parameter, this gets run only once, when the component first renders. The onKeyDown function has a reference to the state at that time, so the keyMap in setKeyMap({ ...keyMap, [e.keyCode]: true }); refers to an empty object. Any time this key handler is called, it sets the state to an empty object, plus the latest key.

Instead, you want it to use the latest state. You have two main options for how to do this

1) Don't skip the effect. This way you'll always have an event listener with the latest state. Remember to return a teardown function so that you don't have an event listener leak:

useEffect(() => {
  const onKeyDown = e => {
    setKeyMap({ ...keyMap, [e.keyCode]: true });
  };

  document.addEventListener('keydown', onKeyDown);
  return () => document.removeEventListener('keydown', onKeyDown);
}); // You could also pass [keyMap] as the second param to skip irrelevant changes

2) Or, use the function version of setKeyMap. It will pass you the latest value of the state.

const onKeyDown = e => {
  setKeyMap(prevKeyMap => ({ ...prevKeyMap, [e.keyCode]: true }));
};
Nicholas Tower
  • 72,740
  • 7
  • 86
  • 98
  • 1
    That's a very good answer. I personally like the second approach since it feels elegant. Are there any performance advantages of using 1st option over the 2nd? – theprogrammer Aug 31 '19 at 06:19
  • 2
    I'd use the second one as well. But it only works because set state supports it. So if you were to run into a similar issue of stale values but no call to set state, you'll want to be aware of the first option. – Nicholas Tower Aug 31 '19 at 06:23
2

You just need to move your onKeyDown function to inside the useEffect hook like this. Note I have added keyMap in the dependency array for the useEffect hook:

https://codesandbox.io/s/red-sun-23bmu?fontsize=14

Jon B
  • 2,444
  • 2
  • 18
  • 19