I have a React/Redux class component which I'm converting to a functional component.

Previously, it had a componentDidMount callback which added an event listener for a custom event dispatched from elsewhere in the app. This is being replaced by useEffect. The event listener, on triggering, calls a method elsewhere in this component.

This method performs actions which depend on the values retrieved from a selector. Initially I was passing useEffect an empty array of dependencies so it would only add the event listener on mount. However, obviously this results in a stale closure around the selector values. A working solution is to pass it the selector as a dependency – however, this results in the listener being removed/re-added every time the value of the selector changes, which isn't great.

I'm trying to think of a solution which only adds the event listener the one time while allowing the called method to access the current value of the selector.

Example code:

const currentValue = useSelector(state => getValue(state));

useEffect(() => {
  document.addEventListener('my.custom.event', handleEvent);

  return(() => document.removeEventListener('my.custom.event', handleEvent));
}, []);

const handleEvent = () => {
  console.log(currentValue)
}

As is, this creates a stale closure around currentValue so that on event trigger, the value logged isn't necessarily the most recent.

Changing [] to [currentValue] in useEffect results in the expected behavior, but removes/re-adds the event listener on every change of currentValue.

Since this isn't a state value of the component, there's no option to use a callback like console.log(currentValue => console.log(currentValue)) to access the most recent value. I also played around with using useRef to hold onto the value, but I believe I'd need some way to update the ref value every time the selector's value changes, which isn't much of a solution.

In the actual component, the value of currentValue is modified in Redux by other components so changing it into a state value isn't really workable either.

I'm wondering:

• whether there's a solution to refreshing the value of the selector in the called method;
• whether the only solution is to remove/re-add the listener on dependency change, or;
• whether it's best to just leave this component as a class component and bypass this issue entirely (componentDidMount doesn't suffer from the stale closure issue.)

CodePudding user response：

The useRef method is usually the solution for this problem, but you'll need another useEffect to update the ref with the currentValue:

const currentValue = useSelector(state => getValue(state));
const valueRef = useRef();

useEffect(() => { valueRef.current = currentValue; }, [currentValue]);

useEffect(() => {
  const handleEvent = () => {
    console.log(valueRef.current)
  }

  document.addEventListener('my.custom.event', handleEvent);

  return(() => document.removeEventListener('my.custom.event', handleEvent));
}, []);

However, you can also extract the entire function out of the useEffect, and use it in a hook, so you can easily create a custom hook for event handling:

const useEventHandler = (eventName, eventHandler, eventTarget = document) => {
  const eventHandlerRef = useRef();

  useEffect(() => {
    eventHandlerRef.current = eventHandler;
  });

  useEffect(() => {
    const handleEvent = (...args) => eventHandlerRef?.current(...args);

    eventTarget.addEventListener(eventName, handleEvent);

    return (() => eventTarget.removeEventListener(eventName, handleEvent));
  }, []);
}

Using the hook:

const currentValue = useSelector(state => getValue(state));

useEventHandler('my.custom.event', () => console.log(currentValue))

CodePudding user response：

however, this results in the listener being removed/re-added every time the value of the selector changes, which isn't great.

Why is it a problem? Does it affect performance of your app in any measurable way? From an immutability point of view, if the value change, then the event listener does something different, so it makes sense to simply replace the event listener by a different one.

If the value changes many times a second (for instance at every frame or every mousemove), then it does create a small performance hit, in which case Ori Drori's solution might be a good one, but it makes the code harder to understand and the performance gain is pretty small.

But if the value only changes a few times a second or less often, I would not worry at all about adding and removing the event listener.