CODEWITHSUNDEEP
javascriptreactjsreact-hooks
simonugor
simonugor

Reputation: 143

ReactJS setState only once

I'm trying to set navbar background colour depending on Y scroll position of a webpage. The problem is once I get in between Y coordinates where I want to setState of my navbar background it won't stop setting it until the page freezes. Here is my code for better understanding:

Getting the Y scroll position, checking if its between 600 & 650, if true setting navbar background colour:

const [navbarBg, setNavbarBg] = useState(style_buttons)

function runOnScroll() {
  const scrolled = window.scrollY
  if (scrolled > 600 && scrolled < 650) {
    setNavbarBg(style_project1)
  }
}

window.addEventListener("scroll", runOnScroll)

It keeps setting the state even when I'm not scrolling and I don't know how to break out of it.

I tried using UseEffect but I don't think I clearly understand how to use it.

Can anyone please help?

Thank you

Upvotes: 1

Views: 1004

Answers (2)

3limin4t0r
3limin4t0r

Reputation: 21110

Your current problem is that you add a new scroll event listener every time the component is rendered. So after the first render you've got 1 event listener, after the second render 2, 3 after the third render etc. This is basically a memory leak because they are never removed.

By using useState you can control how often the event handler is added, but more importantly you can return a cleanup function from useEffect. See the useEffect documentation for more details.

All references to variables that are not defined within the callback itself should be added to the dependency list (second array argument of useState). Unless it is guaranteed that the identity of these variables is stable (the object/value stays the same).

const [navbarBg, setNavbarBg] = useState(style_buttons)

useState(() => {
  function runOnScroll() {
    const scrolled = window.scrollY;
    if (scrolled > 600 && scrolled < 650) {
      setNavbarBg(style_project1);
    }
  }
  window.addEventListener("scroll", runOnScroll);
  return () => window.removeEventListener("scroll", runOnScroll);
}, [style_project1]);

Since we know window and setNavbarBg (see useState) never change they can be left out of the dependency list. However, since you don't provide any info about style_project1 I've added it, just to be sure. If you know this will always hold the same object/value you can change the dependency list to [].

If you don't specify a dependency list things still work, but the callback will be executed after every render. Meaning that each time the component is rendered, the previous scroll event is removed and a new one is added. By adding a dependency list it will only re-run (and cleanup beforehand) when the values in the list change. Which should be a lot more infrequent resulting in better performance.

Upvotes: 1

Sohail Ashraf
Sohail Ashraf

Reputation: 10569

As you mentioned in the comments that the window.addEventListener("scroll", runOnScroll) event listener is before the return function so there would be multiple listener configured as React will run the function body on each re-render.

You need to add the event listener in the useEffect hook and remove/cleanup the listener on component unmount.

In the given code the event listener executes on each render and hence there would be multiple listener getting configured.

Solution:

You could add the listener in useEffect hook and remove the listener in the useEffect callback and pass an empty array as the dependency (to run the hook only once), with this approach there will be only one listener.

const someComponent = function () {

    const [navbarBg, setNavbarBg] = useState(style_buttons)
    useEffect(() => {
        window.addEventListener("scroll", runOnScroll)
        return () => {
            window.removeEventListener("scroll", runOnScroll);
        }
    },[]);
    function runOnScroll() {
        const scrolled = window.scrollY
        if (scrolled > 600 && scrolled < 650) {
            setNavbarBg(style_project1)
        }
    }
    return (
        <>
        </>
    )
}

Upvotes: 1

Related Questions