Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-disabled-scroll-restoration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tanstack/router-core': patch
---

Prevent scroll restoration listeners from being installed when scroll restoration is disabled.
11 changes: 7 additions & 4 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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(
Expand Down
68 changes: 39 additions & 29 deletions packages/router-core/src/scroll-restoration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>()
Expand All @@ -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
}

Expand All @@ -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
}

Expand All @@ -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 (
Expand All @@ -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) {
Expand Down Expand Up @@ -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

Expand Down
84 changes: 84 additions & 0 deletions packages/router-core/tests/scroll-restoration.test.ts
Original file line number Diff line number Diff line change
@@ -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
},
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,9 @@ export const BaseTanStackRouterDevtoolsPanel =
![
'stores',
'basepath',
'injectedHtml',
'subscribers',
'latestLoadPromise',
'navigateTimeout',
'resetNextScroll',
'_scroll',
'tempLocationKey',
'latestLocation',
'routeTree',
Expand Down
Loading