Hey there!
Could you tell me what's wrong with this code?
I have a button in my application that toggles the state.
I would like to achieve the same with pressing the spaΡebar on the keyboard. And it works only one way. The state changes to false once. And then no reaction.
import { useState, useEffect } from 'react'
const HandleKeypress = () => {
const [itWorks, setItWorks] = useState(true)
useEffect(() => {
document.addEventListener('keypress', (e) => {
if (e.code === 'Space') setItWorks(!itWorks)
})
}, [])
return (
<div>
<p>{itWorks ? 'It works!' : 'It does not'}</p>
<button
onClick={() => setItWorks(!itWorks)}
>Press me</button>
</div>
)
}
export default HandleKeypress
What am I missing? πΌ
Top comments (11)
The code within
useEffect(() => {}, [])
is executed only once. The value foritWorks
upon creation of the function istrue
and consequently it will always betrue
.There are two ways you can fix this.
Method 1: update listener each time it changes.
This works because now each time that
itWorks
changes the listener is updated.Method 2: use
setState
with a functionThe function gets the current value in the state so if you're only toggling it you can simply reverse it. This works for all cases where you determine the next state based on the previous state.
Thanks for the insights! With these corrections both methods work indeed!
Is there a reason to prefer one over the other?
The second one causes slightly less work for CPU to do, although the effect is quite minimal with today's computing power. And the work only happens when the value changes, in this case from user action, so not really a big argument for it.
However it is always a good idea to clean up what you create to the DOM, so in this case by adding the removeEventListener part even if using the second approach.
Thank you! Makes sense.
You still want to remove the event listener on unmount in the second example though!
This looks like an issue with stale values inside of
useEffect
.Make this change:
I have created a video on this topic here:
Cheers π»
Thanks Joel! :)
This might work. But haven't tested though.
Thanks Eranda! Seems like it doubles the number of added key listeners with each press and soon the application stops responding.
Which means the answer is to remove keyListeners and clean up useEffect an some point? Not sure how, I've reached the limit of my Hooks understanding π
When we add
itWorks
as a dependency to useEffect, It will re-render for each change ofitWorks
value. So here it will cause the number of event listeners to multiply at each change.So as @Vesa Piittinen suggests it is a good idea to remove the listener if you are adding it in the useEffect hook.
Thanks for the explanation! I appreciate it.