Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useEffect doesn't work for keydown event listener #15815

Closed
zhaoyi0113 opened this issue Jun 4, 2019 · 4 comments
Closed

useEffect doesn't work for keydown event listener #15815

zhaoyi0113 opened this issue Jun 4, 2019 · 4 comments

Comments

@zhaoyi0113
Copy link

I have a stateless component which needs to listen on keyboard event. It adds keydown listener when the component is mounted and remove it when the component is unmounted. There is a state test is boolean value. It is set to true when the component is mounted. But in the keydown event listener, its value always false. It looks like the listener doesn't take the state reference. What's wrong with my code?

const { useEffect, useState } = React;


const Comp = () => {
  const [test, setTest] = useState(false);
  const keyPressHandler = (e) => {
    setTest(!test);
    console.log(test);
  }
  useEffect(() => {
    setTest(true);
    window.addEventListener('keydown', keyPressHandler);
    return () => {
      window.removeEventListener('keydown', keyPressHandler);
    };
  }, []);

  return (
    <div className="test">
      hello {test + ""}
    </div>
  );
};

A live example can be found at: https://codepen.io/zhaoyi0113/pen/mYozVp

@sajalpreetsingh
Copy link

sajalpreetsingh commented Jun 4, 2019

If you remove this ,[] second argument from the useEffect hook in your code, it'll start working. The empty array tells it to run only in the very beginning and end of the component lifecycle (similar to componentDidMount & componentWillUnMount). And, your keyPressHandler was added as a listener only in the first render thus retaining the value of text as "". If you want to use the updated values of text, you should make the suggested change.

Here's a fiddle that works just fine: https://codepen.io/anon/pen/oRVOgK

@Merciful12
Copy link

Merciful12 commented Jun 4, 2019

Also you can put keyPressHandler function inside useEffect body and use setTest to getting previous state not from closure, but from second form with callback.

  const [text, setText] = useState('');
 
  useEffect(() => {
    const keyPressHandler = (e) => {
      setText((text) => text + e.key);
    };

    document.addEventListener('keydown', keyPressHandler);
    return () => {
      document.removeEventListener('keydown', keyPressHandler);
    };
  }, []);

@zhaoyi0113
Copy link
Author

That makes sense, thanks.

@wisnuwijo
Copy link

Also you can put keyPressHandler function inside useEffect body and use setTest to getting previous state not from closure, but from second form with callback.

  const [text, setText] = useState('');
 
  useEffect(() => {
    const keyPressHandler = (e) => {
      setText((text) => text + e.key);
    };

    document.addEventListener('keydown', keyPressHandler);
    return () => {
      document.removeEventListener('keydown', keyPressHandler);
    };
  }, []);

Worked for me, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants