fix(router-core): skip scroll restoration setup when disabled (#7562)

Author: schiller-manuel

.changeset/fix-disabled-scroll-restoration.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/router-core': patch
+---
+
+Prevent scroll restoration listeners from being installed when scroll restoration is disabled.

packages/router-core/src/router.ts

@@ -964,13 +964,16 @@ export class RouterCore<
   tempLocationKey: string | undefined = `${Math.round(
     Math.random() * 10000000,
   )}`
-  resetNextScroll = true
+  _scroll: {
+    next: boolean
+    restoring?: boolean
+    restoration?: boolean
+    reset?: boolean
+  } = { next: true }
   shouldViewTransition?: boolean | ViewTransitionOptions = undefined
   isViewTransitionTypesSupported?: boolean = undefined
   subscribers = new Set<RouterListener<RouterEvent>>()
   viewTransitionPromise?: ControlledPromise<true>
-  isScrollRestoring = false
-  isScrollRestorationSetup = false
 
   // Must build in constructor
   stores!: RouterStores<TRouteTree>
@@ -2227,7 +2230,7 @@ export class RouterCore<
       )
     }
 
-    this.resetNextScroll = next.resetScroll ?? true
+    this._scroll.next = next.resetScroll ?? true
 
     if (!this.history.subscribers.size) {
       this.load(

packages/router-core/src/scroll-restoration.ts

@@ -167,18 +167,17 @@ function getScrollToTopElements(
 
 export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
   // Keep hash/top scrolling active even when sessionStorage is unavailable.
+  const shouldSetupScrollRestoration = force ?? router.options.scrollRestoration
+  const scroll = router._scroll
 
-  if (force ?? router.options.scrollRestoration) {
-    router.isScrollRestoring = true
+  if (shouldSetupScrollRestoration) {
+    scroll.restoring = true
   }
 
-  if ((isServer ?? router.isServer) || router.isScrollRestorationSetup) {
+  if (isServer ?? router.isServer) {
     return
   }
 
-  router.isScrollRestorationSetup = true
-  ignoreScroll = false
-
   const getKey =
     router.options.getScrollRestorationKey || defaultGetScrollRestorationKey
   const trackedScrollEntries = new Map<ScrollTarget, ScrollRestorationEntry>()
@@ -194,10 +193,8 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
     trackedScrollEntries.set(target, entry)
   }
 
-  history.scrollRestoration = 'manual'
-
   const onScroll = (event: Event) => {
-    if (ignoreScroll || !router.isScrollRestoring) {
+    if (ignoreScroll || !scroll.restoring) {
       return
     }
 
@@ -211,7 +208,7 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
 
   // Snapshot the current page's tracked scroll targets before navigation or unload.
   const snapshotCurrentScrollTargets = (restoreKey: string) => {
-    if (!router.isScrollRestoring) {
+    if (!scroll.restoring) {
       return
     }
 
@@ -227,32 +224,45 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
     }
   }
 
-  document.addEventListener('scroll', onScroll, true)
-  router.subscribe('onBeforeLoad', (event) => {
-    if (event.fromLocation) {
-      snapshotCurrentScrollTargets(getKey(event.fromLocation))
-    }
-    trackedScrollEntries.clear()
-  })
-  addEventListener('pagehide', () => {
-    snapshotCurrentScrollTargets(
-      getKey(
-        router.stores.resolvedLocation.get() ?? router.stores.location.get(),
-      ),
-    )
-    persistScrollRestorationCache()
-  })
+  if (shouldSetupScrollRestoration && !scroll.restoration) {
+    scroll.restoration = true
+    ignoreScroll = false
+
+    history.scrollRestoration = 'manual'
+
+    document.addEventListener('scroll', onScroll, true)
+    router.subscribe('onBeforeLoad', (event) => {
+      if (event.fromLocation) {
+        snapshotCurrentScrollTargets(getKey(event.fromLocation))
+      }
+      trackedScrollEntries.clear()
+    })
+    addEventListener('pagehide', () => {
+      snapshotCurrentScrollTargets(
+        getKey(
+          router.stores.resolvedLocation.get() ?? router.stores.location.get(),
+        ),
+      )
+      persistScrollRestorationCache()
+    })
+  }
+
+  if (scroll.reset) {
+    return
+  }
+
+  scroll.reset = true
 
   // Restore destination scroll after the new route has rendered.
   router.subscribe('onRendered', (event) => {
     const behavior = router.options.scrollRestorationBehavior
     const scrollToTopSelectors = router.options.scrollToTopSelectors
-    const shouldResetScroll = router.resetNextScroll
+    const shouldResetScroll = scroll.next
     let scrollToTopElements: Array<Element> | undefined
     trackedScrollEntries.clear()
 
     if (!shouldResetScroll) {
-      router.resetNextScroll = true
+      scroll.next = true
     }
 
     if (
@@ -265,7 +275,7 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
     const cacheKey = getKey(event.toLocation)
     const fromCacheKey = event.fromLocation && getKey(event.fromLocation)
 
-    if (router.isScrollRestoring && fromCacheKey && fromCacheKey !== cacheKey) {
+    if (scroll.restoring && fromCacheKey && fromCacheKey !== cacheKey) {
       const fromElementEntries = scrollRestorationCache[fromCacheKey]
 
       if (fromElementEntries) {
@@ -317,7 +327,7 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) {
           hashScrollIntoViewOptions &&
           (action === 'PUSH' || action === 'REPLACE')
 
-        const elementEntries = router.isScrollRestoring
+        const elementEntries = scroll.restoring
           ? scrollRestorationCache[cacheKey]
           : undefined
 

packages/router-core/tests/scroll-restoration.test.ts

@@ -0,0 +1,84 @@
+import { createMemoryHistory } from '@tanstack/history'
+import { afterEach, describe, expect, test, vi } from 'vitest'
+import { BaseRootRoute, BaseRoute } from '../src'
+import { createTestRouter } from './routerTestUtils'
+
+function createRouter(options: { scrollRestoration?: boolean } = {}) {
+  const rootRoute = new BaseRootRoute({})
+  const indexRoute = new BaseRoute({
+    getParentRoute: () => rootRoute,
+    path: '/',
+  })
+
+  return createTestRouter({
+    routeTree: rootRoute.addChildren([indexRoute]),
+    history: createMemoryHistory({ initialEntries: ['/'] }),
+    ...options,
+  })
+}
+
+afterEach(() => {
+  vi.restoreAllMocks()
+})
+
+describe('setupScrollRestoration', () => {
+  test('sets up scroll restoration when scrollRestoration is true', () => {
+    const windowAddEventListener = vi.spyOn(window, 'addEventListener')
+    const documentAddEventListener = vi.spyOn(document, 'addEventListener')
+    const previousScrollRestoration = window.history.scrollRestoration
+
+    window.history.scrollRestoration = 'auto'
+
+    const router = createRouter({ scrollRestoration: true })
+
+    expect(router._scroll.restoring).toBe(true)
+    expect(router._scroll.restoration).toBe(true)
+    expect(window.history.scrollRestoration).toBe('manual')
+    expect(
+      windowAddEventListener.mock.calls.some(([event]) => event === 'pagehide'),
+    ).toBe(true)
+    expect(
+      documentAddEventListener.mock.calls.some(
+        ([event, _listener, options]) => event === 'scroll' && options === true,
+      ),
+    ).toBe(true)
+
+    window.history.scrollRestoration = previousScrollRestoration
+  })
+
+  test.each([
+    ['omitted', undefined],
+    ['false', false],
+  ] as const)(
+    'does not setup scroll restoration when scrollRestoration is %s',
+    (_name, scrollRestoration) => {
+      const windowAddEventListener = vi.spyOn(window, 'addEventListener')
+      const documentAddEventListener = vi.spyOn(document, 'addEventListener')
+      const previousScrollRestoration = window.history.scrollRestoration
+
+      window.history.scrollRestoration = 'auto'
+
+      const router = createRouter(
+        scrollRestoration === undefined ? {} : { scrollRestoration },
+      )
+
+      expect(router._scroll.restoring).toBeUndefined()
+      expect(router._scroll.restoration).toBeUndefined()
+      expect(router._scroll.reset).toBe(true)
+      expect(window.history.scrollRestoration).toBe('auto')
+      expect(
+        windowAddEventListener.mock.calls.some(
+          ([event]) => event === 'pagehide',
+        ),
+      ).toBe(false)
+      expect(
+        documentAddEventListener.mock.calls.some(
+          ([event, _listener, options]) =>
+            event === 'scroll' && options === true,
+        ),
+      ).toBe(false)
+
+      window.history.scrollRestoration = previousScrollRestoration
+    },
+  )
+})

packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx

@@ -391,11 +391,9 @@ export const BaseTanStackRouterDevtoolsPanel =
               ![
                 'stores',
                 'basepath',
-                'injectedHtml',
                 'subscribers',
                 'latestLoadPromise',
-                'navigateTimeout',
-                'resetNextScroll',
+                '_scroll',
                 'tempLocationKey',
                 'latestLocation',
                 'routeTree',