21

State does get set on the scroll, but logged from the eventlistener, it seems to be stuck at the initial value.

I guess it's something to do with scrolling being set when the side effect's defined, but how could I trigger a state change from a scroll otherwise? Same goes for any window event I presume.

Here's a codesandbox example: https://codesandbox.io/s/react-test-zft3e 

  const [scrolling, setScrolling] = useState(false);

  useEffect(() => {
    window.addEventListener("scroll", () => {
      console.log(scrolling);
      if (scrolling === false) setScrolling(true);
    });
  }, []);

  return (
    <>
      scrolling: {scrolling}
    </>
  );
skyboyer
  • 22,209
  • 7
  • 57
  • 64
Adam Palmer
  • 221
  • 2
  • 3

3 Answers3

35

So your anonymous function is locked on initial value of scrolling. It's how closures works in JS and you better find out some pretty article on that, it may be tricky some time and hooks heavily rely on closures.

So far there are 3 different solutions here:

1. Recreate and re-register handler on each change

useEffect(() => {
    const scrollHandler = () => {
      if (scrolling === false) setScrolling(true);
    };
    window.addEventListener("scroll", scrollHandler);
    return () => window.removeEventListener("scroll", scrollHandler);
  }, [scrolling]);

while following this path ensure your are returning cleanup function from useEffect. It's good default approach but for scrolling it may affect performance because scroll event triggers too often.

2. Access data by reference

const scrolling = useRef(false);

  useEffect(() => {
    const handler = () => {
      if (scrolling.current === false) scrolling.current = true;
    };
    window.addEventListener("scroll", handler);
    return () => window.removeEventListener("scroll", handler);
  }, []);

  return (
    <>
      scrolling: {scrolling}
    </>
  );

downside: changing ref does not trigger re-render. So you need to have some other variable to change it triggering re-render.

3. Use functional version of setter to access most recent value

(I see it as preferred way here):

useEffect(() => {
    const scrollHandler = () => {
      setScrolling((currentScrolling) => {
        if (!currentScrolling) return true;
        return false;
      });
    };
    window.addEventListener("scroll", scrollHandler);
    return () => window.removeEventListener("scroll", scrollHandler);
}, []);

Note Btw even for one-time use effect you better return cleanup function anyway.

PS Also by now you don't set scrolling to false, so you could just get rid of condition if(scrolling === false), but sure in real world scenario you may also run into something alike.

skyboyer
  • 22,209
  • 7
  • 57
  • 64
  • I think the best variant is third. Because first one breaks rules of hooks. https://reactjs.org/docs/hooks-rules.html – Kostya Tresko Mar 05 '20 at 10:24
  • 1
    while I also see 3rd way the best, I don't see how first violate rules of hooks. Could you put some details? – skyboyer Mar 05 '20 at 11:18
  • `Don’t call Hooks inside loops, conditions, or nested functions`. In this case `setScrolling` is called under condition – Kostya Tresko Mar 05 '20 at 14:26
  • 2
    @KostyaTresko no, you misunderstood. You cannot call `if(...) [a] = useState(...)`, but this rule is not applicable to data that hook _returns_. So `setScrolling` can be called anywhere or even passed through the props into child. – skyboyer Mar 05 '20 at 14:36
  • Oh, ok. Thanks for explaining – Kostya Tresko Mar 05 '20 at 15:14
  • Here's that nice little article that explains stale closures https://dmitripavlutin.com/react-hooks-stale-closures/#:~:text=Hooks%20ease%20the%20management%20of,are%20so%20expressive%20and%20simple. – Shabbir Essaji May 25 '22 at 12:06
6

The event listener callback is only initialized once

This means that the variable at that moment are also "trapped" at that point, since on rerender you're not reinitializing the event listener.

It's kind of like a snapshot of the on mount moment.

If you move the console.log outside you will see it change as the rerenders happen and set the scroll value again.

  const [scrolling, setScrolling] = useState(false);

  useEffect(() => {
    window.addEventListener("scroll", () => {
      if (scrolling === false) setScrolling(true);
    });
  }, []);

  console.log(scrolling);

  return (
    <>
      scrolling: {scrolling}
    </>
  );
Joe Lloyd
  • 19,471
  • 7
  • 53
  • 81
0

A solution that has personally served me well when I need to access a state (getState and setState) in an eventListener, without having to create a reference to that state (or all the states it has), is to use the following custom hook:

export function useEventListener(eventName, functionToCall, element) {
  const savedFunction = useRef();

  useEffect(() => {
    savedFunction.current = functionToCall;
  }, [functionToCall]);

  useEffect(() => {
    if (!element) return;
    const eventListener = (event) => savedFunction.current(event);
    element.addEventListener(eventName, eventListener);
    return () => {
      element.removeEventListener(eventName, eventListener);
    };
  }, [eventName, element]);
}

What I do is make a reference to the function to be called in the eventListener. in the component where I need an eventLister, it will look like this:

useEventListener("mousemove", getAndSetState, myRef.current); //myRef.current can be directly the window object 

function getAndSetState() {
    setState(state + 1);
  }

I leave a codesandbox with a more complete code

danielm2402
  • 750
  • 4
  • 14
  • As far as I can tell your hook has a flaw: after the initial render your event handler is not attached because the ref is undefined. A change in the ref will not trigger a re-render. Only if some other state changes that trigger a re-render will more or less accidentally also cause the useEffect to re-evaluate and eventually attach the event handler. You can solve this by returning a `setRef` callabck from your hook that has to be used to obtain the ref. Here is an [example](https://github.com/teetotum/react-panoply/blob/master/src/useEvent/index.js) – Martin Mar 28 '22 at 07:08