diff --git a/.changeset/brave-dingos-hope.md b/.changeset/brave-dingos-hope.md new file mode 100644 index 00000000000..826c01993c8 --- /dev/null +++ b/.changeset/brave-dingos-hope.md @@ -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. diff --git a/e2e/react-start/scroll-restoration/src/router.tsx b/e2e/react-start/scroll-restoration/src/router.tsx index fef35c9e067..81ffc07ad3e 100644 --- a/e2e/react-start/scroll-restoration/src/router.tsx +++ b/e2e/react-start/scroll-restoration/src/router.tsx @@ -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: () => , diff --git a/e2e/react-start/scroll-restoration/src/routes/(tests)/hash-scroll-repro.tsx b/e2e/react-start/scroll-restoration/src/routes/(tests)/hash-scroll-repro.tsx index ff85d140fa6..31970306bdf 100644 --- a/e2e/react-start/scroll-restoration/src/routes/(tests)/hash-scroll-repro.tsx +++ b/e2e/react-start/scroll-restoration/src/routes/(tests)/hash-scroll-repro.tsx @@ -44,9 +44,27 @@ function Component() { > Invalidate + + #one + +
+ {Array.from({ length: 20 }).map((_, i) => ( +
Nested scroll row {i}
+ ))} +
+
{sectionIds.map((sectionId) => (
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) +}) diff --git a/packages/react-router/src/Transitioner.tsx b/packages/react-router/src/Transitioner.tsx index 01abedded98..bf945571a6b 100644 --- a/packages/react-router/src/Transitioner.tsx +++ b/packages/react-router/src/Transitioner.tsx @@ -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' @@ -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]) diff --git a/packages/router-core/src/hash-scroll.ts b/packages/router-core/src/hash-scroll.ts deleted file mode 100644 index ed8354890bf..00000000000 --- a/packages/router-core/src/hash-scroll.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { AnyRouter } from './router' - -/** - * @private - * Handles hash-based scrolling after navigation completes. - * To be used in framework-specific components during the onResolved event. - */ -export function handleHashScroll(router: AnyRouter) { - if (typeof document !== 'undefined' && (document as any).querySelector) { - const location = router.stores.location.get() - const hashScrollIntoViewOptions = - location.state.__hashScrollIntoViewOptions ?? true - - if (hashScrollIntoViewOptions && location.hash !== '') { - const el = document.getElementById(location.hash) - if (el) { - el.scrollIntoView(hashScrollIntoViewOptions) - } - } - } -} diff --git a/packages/router-core/src/index.ts b/packages/router-core/src/index.ts index ded15fff6df..85d04ba0089 100644 --- a/packages/router-core/src/index.ts +++ b/packages/router-core/src/index.ts @@ -406,12 +406,9 @@ export { defaultGetScrollRestorationKey, getElementScrollRestorationEntry, storageKey, - scrollRestorationCache, setupScrollRestoration, } from './scroll-restoration' -export { handleHashScroll } from './hash-scroll' - export type { ScrollRestorationOptions, ScrollRestorationEntry, diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index bd8a2898ec4..04a4a50464e 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -52,6 +52,7 @@ import type { import type { SearchParser, SearchSerializer } from './searchParams' import type { AnyRedirect, ResolvedRedirect } from './redirect' import type { + HistoryAction, HistoryLocation, HistoryState, ParsedHistoryState, @@ -740,7 +741,10 @@ export type GetMatchRoutesFn = (pathname: string) => { export type EmitFn = (routerEvent: RouterEvent) => void -export type LoadFn = (opts?: { sync?: boolean }) => Promise +export type LoadFn = (opts?: { + sync?: boolean + action?: { type: HistoryAction } +}) => Promise export type CommitLocationFn = ({ viewTransition, @@ -883,6 +887,20 @@ export function getLocationChangeInfo( return { fromLocation, toLocation, pathChanged, hrefChanged, hashChanged } } +/** + * Symbol-keyed slot on ParsedLocation carrying the HistoryAction that + * triggered the current load. Read by the scroll-restoration listener to + * decide whether to skip stale window scroll restoration in favor of hash + * navigation. Symbol keys are excluded from Object.keys/JSON/spread, so this + * is invisible to user code. + * @private + */ +export const historyActionKey: unique symbol = Symbol() + +export type ParsedLocationWithHistoryAction = ParsedLocation & { + [historyActionKey]?: HistoryAction +} + export type CreateRouterFn = < TRouteTree extends AnyRoute, TTrailingSlashOption extends TrailingSlashOption = 'never', @@ -2403,7 +2421,8 @@ export class RouterCore< }) } - load: LoadFn = async (opts?: { sync?: boolean }): Promise => { + load: LoadFn = async (opts): Promise => { + const historyAction = opts?.action?.type let redirect: AnyRedirect | undefined let notFound: NotFoundError | undefined let loadPromise: Promise @@ -2415,6 +2434,17 @@ export class RouterCore< this.startTransition(async () => { try { this.beforeLoad() + // Stamp action onto the location instance via symbol key so downstream + // emitters of locationChangeInfo (e.g. onRendered from Match) can read + // it. Only set when an action is present; no-action loads (invalidate, + // same-URL commit, SSR hydration) leave the key absent so that + // deep-equality checks (e.g. toEqual) on location objects are not + // affected by an extraneous Symbol → undefined entry. + if (historyAction !== undefined) { + ;(this.latestLocation as ParsedLocationWithHistoryAction)[ + historyActionKey + ] = historyAction + } const next = this.latestLocation const prevLocation = this.stores.resolvedLocation.get() const locationChangeInfo = getLocationChangeInfo(next, prevLocation) diff --git a/packages/router-core/src/scroll-restoration-inline.ts b/packages/router-core/src/scroll-restoration-inline.ts index b6c772fa693..1e0328a6634 100644 --- a/packages/router-core/src/scroll-restoration-inline.ts +++ b/packages/router-core/src/scroll-restoration-inline.ts @@ -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 { @@ -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] @@ -40,8 +31,8 @@ export default function (options: { window.scrollTo({ top: scrollY, left: scrollX, - behavior: options.behavior, }) + windowRestored = true } else if (elementSelector) { let element @@ -57,10 +48,10 @@ export default function (options: { } } } - - return } + if (windowRestored) return + const hash = window.location.hash.split('#', 2)[1] if (hash) { @@ -77,5 +68,5 @@ export default function (options: { return } - window.scrollTo({ top: 0, left: 0, behavior: options.behavior }) + window.scrollTo({ top: 0, left: 0 }) } diff --git a/packages/router-core/src/scroll-restoration-script/server.ts b/packages/router-core/src/scroll-restoration-script/server.ts index cc7f0f38fbb..d112f144b94 100644 --- a/packages/router-core/src/scroll-restoration-script/server.ts +++ b/packages/router-core/src/scroll-restoration-script/server.ts @@ -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 } @@ -58,7 +50,6 @@ export function getScrollRestorationScriptForRouter(router: AnyRouter) { return getScrollRestorationScript({ storageKey, - shouldScrollRestoration: true, key: userKey, }) } diff --git a/packages/router-core/src/scroll-restoration.ts b/packages/router-core/src/scroll-restoration.ts index a3a89ca1cbc..d4ec57fbdba 100644 --- a/packages/router-core/src/scroll-restoration.ts +++ b/packages/router-core/src/scroll-restoration.ts @@ -1,6 +1,7 @@ import { isServer } from '@tanstack/router-core/isServer' import { functionalUpdate, isPlainObject } from './utils' -import type { AnyRouter } from './router' +import { historyActionKey } from './router' +import type { AnyRouter, ParsedLocationWithHistoryAction } from './router' import type { ParsedLocation } from './location' import type { NonNullableUpdater } from './utils' @@ -76,7 +77,7 @@ function createScrollRestorationCache(): ScrollRestorationCache | null { } } -export const scrollRestorationCache = createScrollRestorationCache() +const scrollRestorationCache = createScrollRestorationCache() /** * The default `getKey` function for `useScrollRestoration`. @@ -140,11 +141,7 @@ const scrollRestorationIdAttribute = 'data-scroll-restoration-id' type ScrollTarget = typeof windowScrollTarget | Element export function setupScrollRestoration(router: AnyRouter, force?: boolean) { - if (!scrollRestorationCache && !(isServer ?? router.isServer)) { - return - } - - const cache = scrollRestorationCache + // Keep hash/top scrolling active even when sessionStorage is unavailable. const shouldScrollRestoration = force ?? router.options.scrollRestoration ?? false @@ -153,11 +150,7 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { router.isScrollRestoring = true } - if ( - (isServer ?? router.isServer) || - router.isScrollRestorationSetup || - !cache - ) { + if ((isServer ?? router.isServer) || router.isScrollRestorationSetup) { return } @@ -195,12 +188,12 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { !router.isScrollRestoring || !restoreKey || trackedScrollEntries.size === 0 || - !cache + !scrollRestorationCache ) { return } - const keyEntry = (cache.state[restoreKey] ||= + const keyEntry = (scrollRestorationCache.state[restoreKey] ||= {} as ScrollRestorationByElement) for (const [target, position] of trackedScrollEntries) { @@ -236,7 +229,7 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { router.stores.resolvedLocation.get() ?? router.stores.location.get(), ), ) - cache.persist() + scrollRestorationCache?.persist() }) // Restore destination scroll after the new route has rendered. @@ -261,10 +254,20 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { ignoreScroll = true try { + const hash = event.toLocation.hash + const hashScrollIntoViewOptions = + event.toLocation.state.__hashScrollIntoViewOptions ?? true + const action = (event.toLocation as ParsedLocationWithHistoryAction)[ + historyActionKey + ] + const skipWindowRestore = + hash && + hashScrollIntoViewOptions && + (action === 'PUSH' || action === 'REPLACE') const elementEntries = router.isScrollRestoring - ? cache.state[cacheKey] + ? scrollRestorationCache?.state[cacheKey] : undefined - let restored = false + let windowRestored = false if (elementEntries) { for (const elementSelector in elementEntries) { @@ -284,12 +287,16 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { } if (elementSelector === windowScrollTarget) { + if (skipWindowRestore) { + continue + } + window.scrollTo({ top: scrollY as number, left: scrollX as number, behavior, }) - restored = true + windowRestored = true } else if (elementSelector) { let element @@ -302,24 +309,17 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { if (element) { element.scrollLeft = scrollX as number element.scrollTop = scrollY as number - restored = true } } } } - if (!restored) { - const hash = router.history.location.hash.slice(1) - + if (!windowRestored) { if (hash) { - const hashScrollIntoViewOptions = - window.history.state?.__hashScrollIntoViewOptions ?? true - if (hashScrollIntoViewOptions) { - const el = document.getElementById(hash) - if (el) { - el.scrollIntoView(hashScrollIntoViewOptions) - } + document + .getElementById(hash) + ?.scrollIntoView(hashScrollIntoViewOptions) } } else { const scrollOptions = { @@ -347,8 +347,8 @@ export function setupScrollRestoration(router: AnyRouter, force?: boolean) { ignoreScroll = false } - if (router.isScrollRestoring) { - cache.set((state) => { + if (router.isScrollRestoring && scrollRestorationCache) { + scrollRestorationCache.set((state) => { state[cacheKey] ||= {} as ScrollRestorationByElement return state }) diff --git a/packages/solid-router/src/Transitioner.tsx b/packages/solid-router/src/Transitioner.tsx index d0983ba1ffe..331aff58642 100644 --- a/packages/solid-router/src/Transitioner.tsx +++ b/packages/solid-router/src/Transitioner.tsx @@ -1,9 +1,5 @@ import * as Solid from 'solid-js' -import { - getLocationChangeInfo, - handleHashScroll, - trimPathRight, -} from '@tanstack/router-core' +import { getLocationChangeInfo, trimPathRight } from '@tanstack/router-core' import { isServer } from '@tanstack/router-core/isServer' import { useRouter } from './useRouter' @@ -133,10 +129,6 @@ export function Transitioner() { router.stores.status.set('idle') router.stores.resolvedLocation.set(router.stores.location.get()) }) - - if (changeInfo.hrefChanged) { - handleHashScroll(router) - } } return currentIsAnyPending diff --git a/packages/vue-router/src/Transitioner.tsx b/packages/vue-router/src/Transitioner.tsx index 66ebbfde389..b0133d965c6 100644 --- a/packages/vue-router/src/Transitioner.tsx +++ b/packages/vue-router/src/Transitioner.tsx @@ -1,9 +1,5 @@ import * as Vue from 'vue' -import { - getLocationChangeInfo, - handleHashScroll, - trimPathRight, -} from '@tanstack/router-core' +import { getLocationChangeInfo, trimPathRight } from '@tanstack/router-core' import { isServer } from '@tanstack/router-core/isServer' import { batch, useStore } from '@tanstack/vue-store' import { useRouter } from './useRouter' @@ -230,10 +226,6 @@ export function useTransitionerSetup() { type: 'onResolved', ...changeInfo, }) - - if (changeInfo.hrefChanged) { - handleHashScroll(router) - } } } catch { // Ignore errors if component is unmounted