Keydown/up events with React Hooks not working properly

Question

I'm trying to create arrow based keyboard controls for a game I'm working on. Of course I'm trying to stay up to date with React so I wanted to create a function component and use hooks. I've created a JSFiddle for my buggy component.

It's almost working as expected, except when I press a lot of the arrow keys at the same time. Then it seems like some keyup events aren't triggered. It could also be that the 'state' is not updated properly.

Which I do like this:

  const ALLOWED_KEYS = ['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight']
  const [pressed, setPressed] = React.useState([])

  const handleKeyDown = React.useCallback(event => {
    const { key } = event
    if (ALLOWED_KEYS.includes(key) && !pressed.includes(key)) {
      setPressed([...pressed, key])
    }
  }, [pressed])

  const handleKeyUp = React.useCallback(event => {
    const { key } = event
    setPressed(pressed.filter(k => k !== key))
  }, [pressed])

  React.useEffect(() => {
    document.addEventListener('keydown', handleKeyDown)
    document.addEventListener('keyup', handleKeyUp)

    return () => {
      document.removeEventListener('keydown', handleKeyDown)
      document.removeEventListener('keyup', handleKeyUp)
    }
  })

I have the idea that I'm doing it correctly, but being new to hooks it is very likely that this is where the problem is. Especially since I've re-created the same component as a class based component: https://jsfiddle.net/vus4nrfe/

And that seems to work fine...

Solution 1

There are 3 key things to do to make it work as expected just like your class component.

As others mentioned for useEffect you need to add an [] as a dependency array which will trigger only once the addEventLister functions.

The second thing which is the main issue is that you are not mutating the pressed array's previous state in functional component as you did in class component, just like below:

// onKeyDown event
this.setState(prevState => ({
   pressed: [...prevState.pressed, key],
}))

// onKeyUp event
this.setState(prevState => ({
   pressed: prevState.pressed.filter(k => k !== key),
}))

You need to update in functional one as the following:

// onKeyDown event
setPressedKeys(previousPressedKeys => [...previousPressedKeys, key]);

// onKeyUp event
setPressedKeys(previousPressedKeys => previousPressedKeys.filter(k => k !== key));

The third thing is that the definition of the onKeyDown and onKeyUp events have been moved inside of useEffect so you don't need to use useCallback.

The mentioned things solved the issue on my end. Please find the following working GitHub repository what I've made which works as expected:

https://github.com/norbitrial/react-keydown-useeffect-componentdidmount

Find a working JSFiddle version if you like it better here:

https://jsfiddle.net/0aogqbyp/

The essential part from the repository, fully working component:

const KeyDownFunctional = () => {
    const [pressedKeys, setPressedKeys] = useState([]);

    useEffect(() => {
        const onKeyDown = ({key}) => {
            if (Consts.ALLOWED_KEYS.includes(key) && !pressedKeys.includes(key)) {
                setPressedKeys(previousPressedKeys => [...previousPressedKeys, key]);
            }
        }

        const onKeyUp = ({key}) => {
            if (Consts.ALLOWED_KEYS.includes(key)) {
                setPressedKeys(previousPressedKeys => previousPressedKeys.filter(k => k !== key));
            }
        }

        document.addEventListener('keydown', onKeyDown);
        document.addEventListener('keyup', onKeyUp);

        return () => {
            document.removeEventListener('keydown', onKeyDown);
            document.removeEventListener('keyup', onKeyUp);
        }
    // eslint-disable-next-line react-hooks/exhaustive-deps
    }, []);

    return <>
        <h3>KeyDown Functional Component</h3>
        <h4>Pressed Keys:</h4>

        {pressedKeys.map(e => <span key={e} className="key">{e}</span>)}
    </>
}

The reason why I'm using // eslint-disable-next-line react-hooks/exhaustive-deps for the useEffect is because I don't want to reattach the events every single time once the pressed or pressedKeys array is changing.

I hope this helps!

Solution 2

User @Vencovsky mentioned the useKeyPress recipe by Gabe Ragland. Implementing this made everything work as expected. The useKeyPress recipe:

// Hook
const useKeyPress = (targetKey) => {
  // State for keeping track of whether key is pressed
  const [keyPressed, setKeyPressed] = React.useState(false)

  // If pressed key is our target key then set to true
  const downHandler = ({ key }) => {
    if (key === targetKey) {
      setKeyPressed(true)
    }
  }

  // If released key is our target key then set to false
  const upHandler = ({ key }) => {
    if (key === targetKey) {
      setKeyPressed(false)
    }
  }

  // Add event listeners
  React.useEffect(() => {
    window.addEventListener('keydown', downHandler)
    window.addEventListener('keyup', upHandler)
    // Remove event listeners on cleanup
    return () => {
      window.removeEventListener('keydown', downHandler)
      window.removeEventListener('keyup', upHandler)
    }
  }, []) // Empty array ensures that effect is only run on mount and unmount

  return keyPressed
}

You can then use that "hook" as follows:

const KeyboardControls = () => {
  const isUpPressed = useKeyPress('ArrowUp')
  const isDownPressed = useKeyPress('ArrowDown')
  const isLeftPressed = useKeyPress('ArrowLeft')
  const isRightPressed = useKeyPress('ArrowRight')

  return (
    <div className="keyboard-controls">
      <div className={classNames('up-button', isUpPressed && 'pressed')} />
      <div className={classNames('down-button', isDownPressed && 'pressed')} />
      <div className={classNames('left-button', isLeftPressed && 'pressed')} />
      <div className={classNames('right-button', isRightPressed && 'pressed')} />
    </div>
  )
}

Complete fiddle can be found here.

The difference with my code is that it use hooks and state per key instead of all the keys at once. I'm not sure why that would matter though. Would be great if somebody could explain that.

Thanks to everyone who tried to help and made the hooks concept clearer for me. And thanks for @Vencovsky for pointing me to the usehooks.com website by Gabe Ragland.

Solution 3

React.useEffect(() => {
  document.addEventListener('keydown', handleKeyDown)
  document.addEventListener('keyup', handleKeyUp)

  return () => {
    document.removeEventListener('keydown', handleKeyDown)
    document.removeEventListener('keyup', handleKeyUp)
  }
}, [handleKeyDown, handleKeyUp]); // <---- Add this deps array

You need to add the handlers as dependencies to the useEffect, otherwise it gets called on every render.

Also, make sure your deps array is not empty [], because your handlers could change based on the value of pressed.

Solution 4

All the solutions I found were pretty bad. For instance, the solutions in this thread only allow you to hold down 2 buttons, or they simply don't work like a lot of the use-hooks libraries.

After working on this for a long time with @asafaviv from #Reactiflux I think this is my favorite solution:

import { useState, useLayoutEffect } from 'react'

const specialKeys = [
  `Shift`,
  `CapsLock`,
  `Meta`,
  `Control`,
  `Alt`,
  `Tab`,
  `Backspace`,
  `Escape`,
]

const useKeys = () => {
  if (typeof window === `undefined`) return [] // Bail on SSR

  const [keys, setKeys] = useState([])

  useLayoutEffect(() => {
    const downHandler = ({ key, shiftKey, repeat }) => {
      if (repeat) return // Bail if they're holding down a key
      setKeys(prevKeys => {
        return [...prevKeys, { key, shiftKey }]
      })
    }
    const upHandler = ({ key, shiftKey }) => {
      setKeys(prevKeys => {
        return prevKeys.filter(k => {
          if (specialKeys.includes(key))
            return false // Special keys being held down/let go of in certain orders would cause keys to get stuck in state
          return JSON.stringify(k) !== JSON.stringify({ key, shiftKey }) // JS Objects are unique even if they have the same contents, this forces them to actually compare based on their contents
        })
      })
    }

    window.addEventListener(`keydown`, downHandler)
    window.addEventListener(`keyup`, upHandler)
    return () => {
      // Cleanup our window listeners if the component goes away
      window.removeEventListener(`keydown`, downHandler)
      window.removeEventListener(`keyup`, upHandler)
    }
  }, [])

  return keys.map(x => x.key) // return a clean array of characters (including special characters 🎉)
}

export default useKeys

Solution 5

I believe you're Breaking the Rules of Hooks:

Do not call Hooks inside functions passed to useMemo, useReducer, or useEffect.

You're calling the setPressed hook inside a function passed to useCallback, which basically uses useMemo under the hood.

useCallback(fn, deps) is equivalent to useMemo(() => fn, deps).

https://reactjs.org/docs/hooks-reference.html#usecallback

See if removing the useCallback in favor of a plain arrow function solves your problem.

View more solutions

Comments

[Vencovsky]

You could take a look at useKeyPress

[thomasjonas]

Good to know that I should use dependencies! It only slightly helped: keys don't get stuck anymore, but it doesn't work for multiple/2 keys at the same time. See updated fiddle :jsfiddle.net/9c85gdxk

[thomasjonas]

Unfortunately that doesn't help. It also doesn't make a difference, but I do believe you're right with me breaking the rules of hooks. I'll keep them plain arrow functions.

[thomasjonas]

Yes, wonderful! Me not using the previous state was the last uncaught problem. (I see you also forgot it in your fiddle for the keyup, here's the final fiddle for completeness: jsfiddle.net/oh8x4smp/1) Thanks for all the time you put in! I think my use of useCallback wasn't necessary at all. Somebody else mentioned it as well. Just an arrow function seems to work fine. Even outside useEffect. Is there any specific reason it has to be inside useEffect?

[norbitrial]

@thomasjonas Great, happy to help! The reason is that you don't need anywhere else, just to attach or detach the event which is handled inside of useEffect. I like to create functions with the minimal scope what I can achieve if I'm not using them anywhere else. I hope this clarifies.

[thomasjonas]

Aha, that makes sense! One more thing: I've noticed a lot of unnecessary re-renders. It seems like !pressedKeys.includes(key) in the onKeyDown handler does not work as the pressedKeys value is not updated. I assume that's because pressedKeys is not in the dependencies array of useEffect, right?

[christopher.theagen]

Could you provide an example of this snippet implemented in a component, please?

[corysimmons]

@cthorpe Actually, I have no idea how to use it... codesandbox.io/s/absolutely-no-idea-budl8 I have no idea what React is doing half the time. I can collect all the keys using that hook, but actually making use of them seems impossible. I ended up using a different approach on my own project that seems to be working where I collect them with a timestamp. If anyone can help debug this! PLEASE DO. I'll update the answer and the demo.

[christopher.theagen]

Thanks- I sympathize! I, too, ended up going a different route using 'react-hotkeys-hook': import { useHotkeys } from 'react-hotkeys-hook'; export default function MyApp({ Component }) { const addBodyClass = cName => document.body.classList.add(cName); const removeBodyClass = cName=> document.body.classList.remove(cName); const setHotkey = (hotkey, cName) => { useHotkeys(hotkey, () => document.body.classList.contains(cName) ? removeBodyClass(cName) : addBodyClass(cName)); }; setHotkey("ctrl+.", 'exampleClass'); return <Component} />; }

[Dan]

I know this is an old post but what if the user wanted manually press the button with a mouse or touch event? You can’t call the function inside of useEffect so how would this be done?