From 93895ccca6dd6933dbef71946c36fb030ee68bd4 Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Sat, 6 Jun 2026 11:19:28 +0200 Subject: [PATCH] fix(router-core): skip scroll restoration setup when disabled --- .changeset/fix-disabled-scroll-restoration.md | 5 ++ packages/router-core/src/router.ts | 11 ++- .../router-core/src/scroll-restoration.ts | 68 ++++++++------- .../tests/scroll-restoration.test.ts | 84 +++++++++++++++++++ .../src/BaseTanStackRouterDevtoolsPanel.tsx | 4 +- 5 files changed, 136 insertions(+), 36 deletions(-) create mode 100644 .changeset/fix-disabled-scroll-restoration.md create mode 100644 packages/router-core/tests/scroll-restoration.test.ts diff --git a/.changeset/fix-disabled-scroll-restoration.md b/.changeset/fix-disabled-scroll-restoration.md new file mode 100644 index 0000000000..357b0aca0b --- /dev/null +++ b/.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. diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 5e3f1d675e..c11cc86082 100644 --- a/packages/router-core/src/router.ts +++ b/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>() viewTransitionPromise?: ControlledPromise - isScrollRestoring = false - isScrollRestorationSetup = false // Must build in constructor stores!: RouterStores @@ -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( diff --git a/packages/router-core/src/scroll-restoration.ts b/packages/router-core/src/scroll-restoration.ts index f3c67802d5..6287a6fce0 100644 --- a/packages/router-core/src/scroll-restoration.ts +++ b/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() @@ -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 | 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 diff --git a/packages/router-core/tests/scroll-restoration.test.ts b/packages/router-core/tests/scroll-restoration.test.ts new file mode 100644 index 0000000000..188d13bb68 --- /dev/null +++ b/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 + }, + ) +}) diff --git a/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx b/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx index a9e804d517..79566656b0 100644 --- a/packages/router-devtools-core/src/BaseTanStackRouterDevtoolsPanel.tsx +++ b/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',