React hooks - setState does not update state properties

Question

I have a event binding from the window object on scroll. It gets properly fired everytime I scroll. Until now everything works fine. But the setNavState function (this is my setState-function) does not update my state properties.

export default function TabBar() {
    const [navState, setNavState] = React.useState(
        {
            showNavLogo: true,
            lastScrollPos: 0
        });

    function handleScroll(e: any) {
        const currScrollPos = e.path[1].scrollY;
        const { lastScrollPos, showNavLogo } = navState;

        console.log('currScrollPos: ', currScrollPos); // updates accordingly to the scroll pos
        console.log('lastScrollPos: ', lastScrollPos); // last scroll keeps beeing 0
        if (currScrollPos > lastScrollPos) {
            setNavState({showNavLogo: false, lastScrollPos: currScrollPos});
        } else {
            setNavState({showNavLogo: true, lastScrollPos: currScrollPos});
        }
    }

    useEffect(() => {
        window.addEventListener('scroll', handleScroll.bind(this));
    }, []);

   ...
   }

So my question is how do I update my state properties with react hooks in this example accordingly?

Answer 1

it's because how closure works. See, on initial render you're declaring handleScroll that has access to initial navState and setNavState through closure. Then you're subscribing for scroll with this #1 version of handleScroll.

Next render your code creates version #2 of handleScroll that points onto up to date navState through closure. But you never use that version for handling scroll.

See, actually it's not your handler "did not update state" but rather it updated it with outdated value.

Option 1

Re-subscribing on each render

useEffect(() => {
  window.addEventListener('scroll', handleScroll);
  return () => window.removeEventListener('scroll', handleScroll);
});

Option 2

Utilizing useCallback to re-create handler only when data is changed and re-subscribe only if callback has been recreated

const handleScroll = useCallback(() => { ... }, [navState]);
useEffect(() => {
  window.addEventListener('scroll', handleScroll);
  return () => window.removeEventListener('scroll', handleScroll);
}, [handleScroll]);

Looks slightly more efficient but more messy/less readable. So I'd prefer first option.

You may wonder why I include navState into dependencies but not setNavState. The reason is - setter(callback returned from useState) is guaranteed to be referentially same on each render.

[UPD] forgot about functional version of setter. It will definitely work fine while we don't want to refer data from another useState. So don't miss up-voting answer by giorgim

Answer 2

Just add dependency and cleanup for useEffect

function TabBar() {
    const [navState, setNavState] = React.useState(
        {
            showNavLogo: true,
            lastScrollPos: 0
        });

    function handleScroll(e) {
        const currScrollPos = e.path[1].scrollY;
        const { lastScrollPos, showNavLogo } = navState;

        console.log('showNavLogo: ', showNavLogo);
        console.log('lastScrollPos: ', lastScrollPos);
        if (currScrollPos > lastScrollPos) {
            setNavState({showNavLogo: false, lastScrollPos: currScrollPos});
        } else {
            setNavState({showNavLogo: true, lastScrollPos: currScrollPos});
        }
    }

    React.useEffect(() => {
        window.addEventListener('scroll', handleScroll.bind(this));
        
        return () => {
          window.removeEventListener('scroll', handleScroll.bind(this));
        }
    }, [navState]);

     return (<h1>scroll example</h1>)
   }
   
   ReactDOM.render(<TabBar />, document.body)

h1 {
  height: 1000px;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.0/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.0/umd/react-dom.production.min.js"></script>