How to add an event listener to useRef in useEffect
Asked Answered
Z

2

16

I'm building a custom hook where I want to add an event listener to a ref, but I'm not sure how to clean up properly, since listRef and listRef.current could be null:

export const myHook: MyHook = () => {
  const listRef = useRef<HTMLDivElement>(null)

  useEffect(() => {
    // I can check for the presence of `listRef.current` here ...
    if (listRef && listRef.current) {
      listRef.current.addEventListener(...)
    }

    // ... but what's the right way for the return function?
    return listRef.current.removeEventListener(...)
  })

  return [listRef]
}
Zacynthus answered 3/3, 2020 at 18:5 Comment(0)
T
13

Edit:

but I have to check for the presence of listRef in the return function too, right?

Yes, and what you could do is wrap everythinig around the if statement

  useEffect(() => {
    // Everything around if statement
    if (listRef && listRef.current) {
      listRef.current.addEventListener(...)
    
      return () => {
        listRef.current.removeEventListener(...)
      }
    }
  }, [listRef])

If you don't call addEventListener, you don't need to call removeEventListener, so that is why you put everything inside the if.


You need to pass a function in the return that do what you want to do in the cleanup.

export const myHook: MyHook = () => {
  const listRef = useRef<HTMLDivElement>(null)

  useEffect(() => {
    // This is ok
    if (listRef && listRef.current) {
      listRef.current.addEventListener(...)
    }

    // Passing a function that calls your function
    return () => {
        listRef.current.removeEventListener(...)
    }
  }, [listRef])

  return [listRef]
}

Another thing you need to notice is that inside the fooEventListener, the ... should be the same reference of the function, this means:

You shoudn't do this:

  useEffect(() => {
    if (listRef && listRef.current) {
      listRef.current.addEventListener(() => console.log('do something'))
    }

    return () => {
        listRef.current.removeEventListener(() => console.log('do something'))
    }
  })

And you should do :

  useEffect(() => {
    const myFunction = () => console.log('do something')

    if (listRef && listRef.current) {
      // Passing the same reference
      listRef.current.addEventListener(myFunction)
    }

    return () => {
      // Passing the same reference
      listRef.current.removeEventListener(myFunction)
    }
  }, [listRef])
Twila answered 3/3, 2020 at 18:8 Comment(8)
Oh, thanks, but I have to check for the presence of listRef in the return function too, right?Zacynthus
Thanks, I didn't know that a conditional return statement for this is fine.Zacynthus
This should not be accepted as the right answer. useEffect has no dependency list and will be adding / removing event listeners every render, which is very wrong.Erythropoiesis
@WyattArent that makes sense, and a solution would be adding a empty array as a dependency list. But the OP didn't mentioned any of that. I don't know what's the ... in addEventListener(...). Maybe the OP wants to add and remove on every render? I don't know and there's no mention of it, so I just leaved that the way the OP made the question (without []).Twila
@WyattArent there's nothing wrong with that. If the OP wants and accepts without an empty dependency array, then it's right for the OP. Maybe for you or other cases it's not. But how can I know? ¯\_(ツ)_/¯Twila
@Twila We wouldn't know but if "how do I put off the fire coming out of my deep fryer?" is answered by "pour some water on it", and accepted by the OP(assuming it worked on OP's case), that doens't make it right.Learning
@Learning you are assuming that the answer provided should answer only the question in the title, which is not true. You need to take in count for everything in the answer, including the code from OP. I can't make assumptions on what is in ... from the code. But the answer that goes beyond the OP's question is already answered in one of my previous commentsTwila
This is not the right answer. After the first render listRef.current will be set, but this doesn't cause a render. That's why useEffect will not be called.Geophysics
B
0

It is unsafe to do this in fact. Try this example:

import React from 'react';
import {useEffect, useRef, useState} from 'react'

export function App(props) {
  const ref = useRef()
  const [cnt, setCnt] = useState(1)
  useEffect(() => {
  const handleClick = event => {
    console.log("dsadsa")
    setCnt(prev=>prev+1)
  };
  const element = ref.current;
  element.addEventListener('click', handleClick);
  return () => {
    element.removeEventListener('click', handleClick);
  };
}, []);
  return (
    <div className='App'>
      <div key={Math.random()} ref={ref}>dasdsa{cnt}</div>
    </div>
  );
}

You will see the event listener can only execute once. I guess this is not your designated behavior.

When re-render, if the underline Dom is changed, react won't help you to move your event listener into the new Dom. Do careful for binding events to ref...

Belgrade answered 11/2 at 19:49 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.