Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 8 additions & 0 deletions .changeset/brave-dingos-hope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@tanstack/router-core': patch
'@tanstack/react-router': patch
'@tanstack/solid-router': patch
'@tanstack/vue-router': patch
---

Fix hash navigation being overridden by stale scroll restoration entries.
7 changes: 7 additions & 0 deletions e2e/react-start/scroll-restoration/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ export function getRouter() {
const router = createRouter({
routeTree,
scrollRestoration: true,
getScrollRestorationKey: (location) => {
if (location.pathname === '/hash-scroll-repro') {
return location.pathname
}

return location.state.__TSR_key! || location.href
},
defaultPreload: 'intent',
defaultErrorComponent: DefaultCatchBoundary,
defaultNotFoundComponent: () => <NotFound />,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,27 @@ function Component() {
>
Invalidate
</button>
<Link
to="/hash-scroll-repro"
hash="one"
className="rounded border px-3 py-2"
data-testid="hash-scroll-section-one-link"
>
#one
</Link>
</div>
</div>

<div
data-scroll-restoration-id="hash-scroll-nested"
data-testid="hash-scroll-nested"
className="mt-4 h-24 overflow-auto rounded border p-2"
>
{Array.from({ length: 20 }).map((_, i) => (
<div key={i}>Nested scroll row {i}</div>
))}
</div>

<div className="mt-6 grid gap-10">
{sectionIds.map((sectionId) => (
<section
Expand Down
62 changes: 62 additions & 0 deletions e2e/react-start/scroll-restoration/tests/hash-scroll-repro.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,65 @@ test('router.invalidate does not scroll back to the current hash', async ({
const scrollYAfterInvalidate = await page.evaluate(() => window.scrollY)
expect(scrollYAfterInvalidate).toBe(scrollYBeforeInvalidate)
})

test('hash navigation wins over stale same-tab scroll restoration entries', async ({
page,
}) => {
await goToRepro(page)
const staleScrollY = await scrollUpFromHashTarget(page)

await page.reload()
await page.waitForLoadState('networkidle')
await expect(
page.getByTestId('hash-scroll-repro-invalidate-count'),
).toBeVisible()

await page.getByTestId('hash-scroll-section-one-link').click()
await expect(page.getByTestId('hash-scroll-section-one')).toBeInViewport()

await expect(
page.getByTestId('hash-scroll-section-five'),
).not.toBeInViewport()

const scrollYAfterHashNavigation = await page.evaluate(() => window.scrollY)
expect(scrollYAfterHashNavigation).toBeLessThan(staleScrollY)
})

test('hash navigation still runs when only nested scroll entries restore', async ({
page,
}) => {
await goToRepro(page)

const nestedScrollTop = await page.evaluate(() => {
const nested = document.querySelector('[data-testid="hash-scroll-nested"]')
if (!(nested instanceof HTMLElement)) {
throw new Error('Missing nested scroller')
}

nested.scrollTop = 80
window.dispatchEvent(new PageTransitionEvent('pagehide'))
return nested.scrollTop
})

expect(nestedScrollTop).toBeGreaterThan(0)

await page.reload()
await page.waitForLoadState('networkidle')
await expect(
page.getByTestId('hash-scroll-repro-invalidate-count'),
).toBeVisible()

await page.getByTestId('hash-scroll-section-one-link').click()
await expect(page.getByTestId('hash-scroll-section-one')).toBeInViewport()

await expect
.poll(async () => {
return page.evaluate(() => {
const nested = document.querySelector(
'[data-testid="hash-scroll-nested"]',
)
return nested instanceof HTMLElement ? nested.scrollTop : 0
})
})
.toBe(nestedScrollTop)
})
10 changes: 1 addition & 9 deletions packages/react-router/src/Transitioner.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@

import * as React from 'react'
import { batch, useStore } from '@tanstack/react-store'
import {
getLocationChangeInfo,
handleHashScroll,
trimPathRight,
} from '@tanstack/router-core'
import { getLocationChangeInfo, trimPathRight } from '@tanstack/router-core'
import { useLayoutEffect, usePrevious } from './utils'
import { useRouter } from './useRouter'

Expand Down Expand Up @@ -128,10 +124,6 @@ export function Transitioner() {
router.stores.status.set('idle')
router.stores.resolvedLocation.set(router.stores.location.get())
})

if (changeInfo.hrefChanged) {
handleHashScroll(router)
}
}
}, [isAnyPending, previousIsAnyPending, router])

Expand Down
21 changes: 0 additions & 21 deletions packages/router-core/src/hash-scroll.ts

This file was deleted.

3 changes: 0 additions & 3 deletions packages/router-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,12 +406,9 @@ export {
defaultGetScrollRestorationKey,
getElementScrollRestorationEntry,
storageKey,
scrollRestorationCache,
setupScrollRestoration,
} from './scroll-restoration'

export { handleHashScroll } from './hash-scroll'

export type {
ScrollRestorationOptions,
ScrollRestorationEntry,
Expand Down
34 changes: 31 additions & 3 deletions packages/router-core/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import type {
import type { SearchParser, SearchSerializer } from './searchParams'
import type { AnyRedirect, ResolvedRedirect } from './redirect'
import type {
HistoryAction,
HistoryLocation,
HistoryState,
ParsedHistoryState,
Expand Down Expand Up @@ -573,6 +574,7 @@ export interface BuildNextOptions {
type NavigationEventInfo = {
fromLocation?: ParsedLocation
toLocation: ParsedLocation
historyAction?: HistoryAction
pathChanged: boolean
hrefChanged: boolean
hashChanged: boolean
Expand Down Expand Up @@ -740,7 +742,10 @@ export type GetMatchRoutesFn = (pathname: string) => {

export type EmitFn = (routerEvent: RouterEvent) => void

export type LoadFn = (opts?: { sync?: boolean }) => Promise<void>
export type LoadFn = (opts?: {
sync?: boolean
action?: { type: HistoryAction }
}) => Promise<void>

export type CommitLocationFn = ({
viewTransition,
Expand Down Expand Up @@ -877,10 +882,31 @@ export function getLocationChangeInfo(
) {
const fromLocation = resolvedLocation
const toLocation = location
const historyAction = locationHistoryActions.get(toLocation)
const pathChanged = fromLocation?.pathname !== toLocation.pathname
const hrefChanged = fromLocation?.href !== toLocation.href
const hashChanged = fromLocation?.hash !== toLocation.hash
return { fromLocation, toLocation, pathChanged, hrefChanged, hashChanged }
return {
fromLocation,
toLocation,
historyAction,
pathChanged,
hrefChanged,
hashChanged,
}
}

const locationHistoryActions = new WeakMap<ParsedLocation, HistoryAction>()

function setLocationHistoryAction(
location: ParsedLocation,
action: HistoryAction | undefined,
) {
if (action) {
locationHistoryActions.set(location, action)
} else {
locationHistoryActions.delete(location)
}
}

export type CreateRouterFn = <
Expand Down Expand Up @@ -2403,7 +2429,8 @@ export class RouterCore<
})
}

load: LoadFn = async (opts?: { sync?: boolean }): Promise<void> => {
load: LoadFn = async (opts): Promise<void> => {
const historyAction = opts?.action?.type
let redirect: AnyRedirect | undefined
let notFound: NotFoundError | undefined
let loadPromise: Promise<void>
Expand All @@ -2415,6 +2442,7 @@ export class RouterCore<
this.startTransition(async () => {
try {
this.beforeLoad()
setLocationHistoryAction(this.latestLocation, historyAction)
const next = this.latestLocation
const prevLocation = this.stores.resolvedLocation.get()
const locationChangeInfo = getLocationChangeInfo(next, prevLocation)
Expand Down
23 changes: 7 additions & 16 deletions packages/router-core/src/scroll-restoration-inline.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
export default function (options: {
storageKey: string
key?: string
behavior?: ScrollToOptions['behavior']
shouldScrollRestoration?: boolean
}) {
export default function (options: { storageKey: string; key?: string }) {
let byKey

try {
Expand All @@ -15,13 +10,9 @@ export default function (options: {

const resolvedKey = options.key || window.history.state?.__TSR_key
const elementEntries = resolvedKey ? byKey[resolvedKey] : undefined
let windowRestored = false

if (
options.shouldScrollRestoration &&
elementEntries &&
typeof elementEntries === 'object' &&
Object.keys(elementEntries).length > 0
) {
if (elementEntries && typeof elementEntries === 'object') {
for (const elementSelector in elementEntries) {
const entry = elementEntries[elementSelector]

Expand All @@ -40,8 +31,8 @@ export default function (options: {
window.scrollTo({
top: scrollY,
left: scrollX,
behavior: options.behavior,
})
windowRestored = true
} else if (elementSelector) {
let element

Expand All @@ -57,10 +48,10 @@ export default function (options: {
}
}
}

return
}

if (windowRestored) return

const hash = window.location.hash.split('#', 2)[1]

if (hash) {
Expand All @@ -77,5 +68,5 @@ export default function (options: {
return
}

window.scrollTo({ top: 0, left: 0, behavior: options.behavior })
window.scrollTo({ top: 0, left: 0 })
}
11 changes: 1 addition & 10 deletions packages/router-core/src/scroll-restoration-script/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,18 @@ import type { AnyRouter } from '../router'
type InlineScrollRestorationScriptOptions = {
storageKey: string
key?: string
behavior?: ScrollToOptions['behavior']
shouldScrollRestoration?: boolean
}

const defaultInlineScrollRestorationScript = `(${minifiedScrollRestorationScript})(${escapeHtml(
JSON.stringify({
storageKey,
shouldScrollRestoration: true,
} satisfies InlineScrollRestorationScriptOptions),
)})`

function getScrollRestorationScript(
options: InlineScrollRestorationScriptOptions,
) {
if (
options.storageKey === storageKey &&
options.shouldScrollRestoration === true &&
options.key === undefined &&
options.behavior === undefined
) {
if (options.storageKey === storageKey && options.key === undefined) {
return defaultInlineScrollRestorationScript
}

Expand Down Expand Up @@ -58,7 +50,6 @@ export function getScrollRestorationScriptForRouter(router: AnyRouter) {

return getScrollRestorationScript({
storageKey,
shouldScrollRestoration: true,
key: userKey,
})
}
Loading
Loading