Unavoidable eslintreact-hooks/exhaustive-deps warning after restoring scroll position from session storage

[piotrpawlik]

My implementation for restoring scroll position after route change goes like this:

import { useEffect, useRef } from 'react'
import { useLocation } from 'react-router-dom'
import { useScroll, useSessionStorage } from 'react-use'

export const useRestoreScrollOnRouteChange = (storageKey: string) => {
  const ref = useRef<HTMLDivElement>(null)
  const { y } = useScroll(ref)
  const { pathname } = useLocation()
  const scrollPositionRestoredRef = useRef(false)
  const [storedScroll, storeScroll] = useSessionStorage<number>(storageKey, 0)

  useEffect(() => {
    ref.current?.scrollTo({ top: storedScroll })
    scrollPositionRestoredRef.current = true
  }, [pathname]) // scroll position restoration should happen only on route change - eslintreact-hooks/exhaustive-deps warns about this line

  useEffect(() => {
    if (scrollPositionRestoredRef.current) {
      storeScroll(y)
    }
  }, [y])

  return ref
}

I had to omit storedScroll from dependencies of first useEffect since I do want "restoring" to be triggered only when route changes.

I went through entire https://reactjs.org/docs/hooks-faq.html#is-it-safe-to-omit-functions-from-the-list-of-dependencies section and got encourage by docs to create this issue although I don't see any alternative implementation that wouldn't fire warning.

At the end, storedScroll is dependent on sessionStorage not state nor props (although internally in useSessionStorage it is stored as state).

[marko-knoebl]

At the end, storedScroll is dependent on sessionStorage not state nor props (although internally in useSessionStorage it is stored as state).

I think that's the root cause of the issue here - you're using a React state internally where you wouldn't need one.

I think you could just replace:

storeScroll(y) -> sessionStorage.setItem(storageKey, y)

and inside your first effect: const storedScroll = Number(sessionStorage.getItem(storageKey))

Then you won't need the extra hook

[eps1lon]

Closing in favor of #15240 which talks about the same underlying issue.

There was an RFC recently published that might help with this issue: reactjs/rfcs#220