Add Scroll Event Listener to Window Slows Down Scroll

Question

My webpage has a scroll event listener, which detects where the user is on the page. Basically, this functionality is to enlarge a nav bar or not.

The event listener is straightforward, and looks like this:

MyComponent.jsx

const MyComponent = () => {
  const { setWindowHeight } = useContext(AppContext);

  const handleScroll = useCallback(
    (div) => {
      setWindowHeight(div.scrollTop) // the set state is coming from a context provider shared across the app
    },
    []
  );

  // Attach the scroll listener to the div
  useEffect(() => {
    const scrollableElement =
      document.getElementsByClassName('scrollItem')[0];
    scrollableElement.addEventListener('scroll', () => handleScroll(scrollableElement));
    }
  }, [handleScroll]);

  return (
    // The component
  )
}

NavBar.jsx

const NavBar = () => {
  const { windowHeight } = useContext(AppContext);

  return (
    <div style={{minHeight: windowHeight > 0 ? '5vh' : '20vh'}}>
      // The navbar
    </div>
  )
}

However, my problem is that this has slowed down my page quite a bit, and every scroll seems to lag, presumably because there is a re-render on every scroll to figure out where the user is on the page.

Is there a better way to optimize this?

Are you setting any state inside `handleScroll` ? it would be helpful if you can provide a sample code on how you are planning to handle the event. — Ahsan-J, Mar 28 '23 at 10:38
Could you knock up a working snippet showing this, there is nothing intrinsically with anything in your code that says slow. — Keith, Mar 28 '23 at 10:38
@Ahsan-J I am indeed setting a state inside `handleScroll`. I will edit my post with the code, but it's basically setting the window height. In my `NavBar` component, I retrieve the window height (through a context provider) and depending on its value, resizes it to bigger/smaller — Adam, Mar 28 '23 at 10:45
Be sure to define the handler _outside_ `useEffect`, as you will add a new event listener every time a render occurs, slowly stacking them up, making it progressively slower. `() => handleScroll()` is _unique every time you add it_, resulting in many listeners as it can't distinguish. `const handle = () => handleScroll; useEffect(){ addEventListener( 'scroll', handle )}` will be just _one_ event listener, executed once. — somethinghere, Mar 28 '23 at 10:50
Ah yes, like @somethinghere has pointed out your stacking up events, in React if you use `addEventListener` you really want to pair this up with `removeEventListener` inside your unmount callback -> `return () => removeEventListener(onScroll)`. — Keith, Mar 28 '23 at 10:52
@Keith Or that, that works too. Forgot about the cleanup method. — somethinghere, Mar 28 '23 at 10:53
Thank you both. I've edited my original post before your comments. My `handleScroll` is in a `useCallback`. Would that be the same as your suggestion @somethinghere? — Adam, Mar 28 '23 at 10:56
I'm not very familiar with `useCallback`, does it rate limit them? Personally, I would just add the listener and remove it in the unmount as Keith mentioned, it's the easiest to ensure you don't get duplicates. BUT it's rather important that you then first define it as a `const` (say `const handler = () => handleScroll(scrollableElement);` inside `useEffect`, then `addEventListener( 'scroll', handler )` and `removeEventListener( 'scroll', handler )` in the unmount.) It's important that add and remove _point to exactly the same method_ to prevent duplications. — somethinghere, Mar 28 '23 at 10:59
@somethinghere thanks for clarifying. Would it be okay to show an example (perhaps as an answer) of what you mean? I am using Typescript and if I make a reference to a method within a `useEffect` to which that method isn't wrapped around a `useCallback`, I get a warning: `The 'handleScroll' function makes the dependencies of useEffect Hook (at line 416) change on every render. Move it inside the useEffect callback` — Adam, Mar 28 '23 at 11:09
I've added it and hope this will work for you. Let me know if it does! — somethinghere, Mar 28 '23 at 11:14

somethinghere · Answer 1 · 2023-03-28T11:21:23.030

1

As requested, I would structure the code as such to prevent the event listener from being added multiple times:

const MyComponent = () => {

  const { setWindowHeight } = useContext(AppContext);
  const handleScroll = useCallback(div => setWindowHeight(div.scrollTop), []);

  useEffect(() => {

    const scrollableElement = document.getElementsByClassName('scrollItem')[0];
    // Create a function instance that we can uniquely reference
    const handler = () => handleScroll( scrollableElement );

    // Attach the handler
    scrollableElement.addEventListener('scroll', handler);

    // On unmount, detach the handler
    return () => scrollableElement.removeEventListener('scroll', handler);

  }, [handleScroll]);

  return ( /* The component */ );

}

The issue you probably had is that every time the component was mounted or renderer it would add another instance of a listener. That's because every () => {} is technically a unique method, even if it executes the same code. It would also keep a reference to the old HTMLElement around, which means it can't be removed from memory. This will slowly add up. Event listeners also don't know what they add, they just add every unique function once. Consider this:

const method = () => console.log( 'I did something' );

console.log( method !== (() => method()) ); // false, the (() => method()) is a unique, different method that calls the other method.

That's why you should just construct the method you want to pass to add and remove for eventListener, because:

console.log( (() => method()) === (() => method()) ); // These objects are not the same!

edited Mar 28 '23 at 11:21

answered Mar 28 '23 at 11:13

somethinghere

16,311
2
28
42

Thank you so much! The scrolling is still a bit laggy but it works better than my original solution. – Adam Mar 28 '23 at 11:21
1

@Adam I wouldn't want to hijack the question, tbh, if you don't think this is the final solution and it's still laggy it might be good to not mark my answer as the right one (I don't mind the reputation, but I'd rather you get to solve your problem). It would be a little unnecessary to add another question to SO with basically the same contents... – somethinghere Mar 28 '23 at 11:22
1

You're right. I've unmarked it for now just in case I can resolve this issue completely. Is it possible to invoke the handler to scroll only after x ms instead of continuously? – Adam Mar 28 '23 at 11:24
It is possible to do such a thing with a `debouncer` , for that maybe look here: https://stackoverflow.com/questions/24004791/what-is-the-debounce-function-in-javascript – somethinghere Mar 28 '23 at 11:25
1

Although the debouncer might not do that exactly. You could debounce it before the addEventListener is triggered for the actual scroll. Mainly because a debouncer just waits until an event is `x` amount of time in the past and not triggered again in that time. It could help, but it means it updates only x amount of time after your scroll. You might want to figure out what `setWindowHeight` does and whether it's just slow. Perhaps a `ResizeObserver` is more useful for setting the window height? Not sure... it depends on what you use it for. – somethinghere Mar 28 '23 at 11:30
1

I think you're right. It may be the `setWindowHeight` that is causing it to be very slow. And you're also right with regards to the update - I'd like it to be near instantaneous so adding a debouncer might not work in the end. I'll have a look at `ResizeObserver` in that case too. Thank you once again for all your help! – Adam Mar 28 '23 at 11:34

Add Scroll Event Listener to Window Slows Down Scroll

1 Answers1