fix(nav): scroll-restore key auf nav.from.url, nicht location
All checks were successful
Build & Publish Docker Image / build-and-push (push) Successful in 2m51s

Live-Test auf kochwas-dev offenbarte: bei Browser-Back hat der Browser
die History bereits gepoppt, bevor SvelteKit beforeNavigate feuert —
location.pathname war damit schon die Ziel-URL. recordScroll() schrieb
also den 0-Wert der Recipe-Page in den Slot der Home-Page und wischte
den eigentlich gemerkten Wert (z. B. 500) raus. Restore las dann 0,
fiel unter MIN_RESTORE_Y und tat nichts.

Fix: recordScroll(nav.from?.url) und restoreScroll(type, nav.to?.url).
Helper bekommen die URL explizit reingereicht — keine Abhängigkeit
mehr von location und damit kein Race mit der Browser-History.

Tests: zusätzliche Regression "does not overwrite a stored URL when
called with a different from-url" plus Skip-Pfade fuer null URLs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
hsiegeln
2026-04-22 09:28:20 +02:00
parent 442076a278
commit f3e2cebfb4
3 changed files with 57 additions and 32 deletions

View File

@@ -5,9 +5,11 @@
// wishlist, shopping-list) are still empty at that point, so the saved // wishlist, shopping-list) are still empty at that point, so the saved
// scrollY can't be reached and the browser clamps to 0. // scrollY can't be reached and the browser clamps to 0.
// //
// We patch this by saving scrollY on beforeNavigate and re-applying it // We patch this by saving scrollY on beforeNavigate (keyed by the URL
// after popstate as soon as the document is tall enough — using rAF // we're leaving — NOT location.pathname, which on popstate is already
// polling with a hard time budget so we never spin. // the new URL by the time the callback fires) and re-applying it after
// popstate as soon as the document is tall enough — rAF-polled with a
// hard time budget so we never spin.
const STORAGE_KEY = 'kochwas:scroll'; const STORAGE_KEY = 'kochwas:scroll';
const POLL_BUDGET_MS = 1500; const POLL_BUDGET_MS = 1500;
@@ -34,26 +36,26 @@ function writeMap(map: ScrollMap): void {
} }
} }
function urlKey(): string { function keyFor(url: URL): string {
if (typeof location === 'undefined') return ''; return url.pathname + url.search;
return location.pathname + location.search;
} }
export function recordScroll(): void { export function recordScroll(fromUrl: URL | null | undefined): void {
if (typeof window === 'undefined') return; if (typeof window === 'undefined') return;
const key = urlKey(); if (!fromUrl) return;
if (!key) return;
const map = readMap(); const map = readMap();
map[key] = window.scrollY; map[keyFor(fromUrl)] = window.scrollY;
writeMap(map); writeMap(map);
} }
export function restoreScroll(navType: string | null | undefined): void { export function restoreScroll(
navType: string | null | undefined,
toUrl: URL | null | undefined
): void {
if (typeof window === 'undefined') return; if (typeof window === 'undefined') return;
if (navType !== 'popstate') return; if (navType !== 'popstate') return;
const key = urlKey(); if (!toUrl) return;
if (!key) return; const target = readMap()[keyFor(toUrl)];
const target = readMap()[key];
if (!target || target < MIN_RESTORE_Y) return; if (!target || target < MIN_RESTORE_Y) return;
const start = performance.now(); const start = performance.now();

View File

@@ -98,8 +98,8 @@
} }
} }
beforeNavigate(() => { beforeNavigate((nav) => {
recordScroll(); recordScroll(nav.from?.url);
}); });
afterNavigate((nav) => { afterNavigate((nav) => {
@@ -112,7 +112,7 @@
// wurde. // wurde.
void wishlistStore.refresh(); void wishlistStore.refresh();
void shoppingCartStore.refresh(); void shoppingCartStore.refresh();
restoreScroll(nav.type); restoreScroll(nav.type, nav.to?.url);
}); });
onMount(() => { onMount(() => {

View File

@@ -20,6 +20,10 @@ function setViewportHeight(h: number) {
Object.defineProperty(window, 'innerHeight', { value: h, configurable: true }); Object.defineProperty(window, 'innerHeight', { value: h, configurable: true });
} }
function url(path: string): URL {
return new URL(path, 'https://example.test');
}
describe('scroll-restore', () => { describe('scroll-restore', () => {
let scrollToSpy: ReturnType<typeof vi.spyOn>; let scrollToSpy: ReturnType<typeof vi.spyOn>;
@@ -31,7 +35,6 @@ describe('scroll-restore', () => {
scrollToSpy = vi scrollToSpy = vi
.spyOn(window, 'scrollTo') .spyOn(window, 'scrollTo')
.mockImplementation(() => undefined as unknown as void); .mockImplementation(() => undefined as unknown as void);
window.history.replaceState({}, '', '/wishlist');
}); });
afterEach(() => { afterEach(() => {
@@ -39,64 +42,84 @@ describe('scroll-restore', () => {
vi.useRealTimers(); vi.useRealTimers();
}); });
it('records scrollY keyed by pathname+search', () => { it('records scrollY keyed by from-url pathname+search', () => {
setScrollY(1200); setScrollY(1200);
recordScroll(); recordScroll(url('/wishlist'));
const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}'); const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}');
expect(map['/wishlist']).toBe(1200); expect(map['/wishlist']).toBe(1200);
}); });
it('keeps separate entries per URL', () => { it('keeps separate entries per URL', () => {
setScrollY(500); setScrollY(500);
recordScroll(); recordScroll(url('/wishlist'));
window.history.replaceState({}, '', '/?q=hi');
setScrollY(900); setScrollY(900);
recordScroll(); recordScroll(url('/?q=hi'));
const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}'); const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}');
expect(map['/wishlist']).toBe(500); expect(map['/wishlist']).toBe(500);
expect(map['/?q=hi']).toBe(900); expect(map['/?q=hi']).toBe(900);
}); });
it('does not overwrite a stored URL when called with a different from-url', () => {
// This is the regression: on popstate, location.pathname is already
// the new URL. Recording must use nav.from.url (the page being left),
// not location, or we wipe the destination's saved scrollY.
setScrollY(500);
recordScroll(url('/'));
setScrollY(0);
recordScroll(url('/recipes/1'));
const map = JSON.parse(sessionStorage.getItem(STORAGE_KEY) ?? '{}');
expect(map['/']).toBe(500);
expect(map['/recipes/1']).toBe(0);
});
it('skips when from-url is missing', () => {
setScrollY(900);
recordScroll(null);
expect(sessionStorage.getItem(STORAGE_KEY)).toBeNull();
});
it('skips restore for non-popstate navigation', () => { it('skips restore for non-popstate navigation', () => {
sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 })); sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 }));
setDocHeight(5000); setDocHeight(5000);
restoreScroll('link'); restoreScroll('link', url('/wishlist'));
expect(scrollToSpy).not.toHaveBeenCalled();
});
it('skips restore when to-url is missing', () => {
sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 }));
restoreScroll('popstate', null);
expect(scrollToSpy).not.toHaveBeenCalled(); expect(scrollToSpy).not.toHaveBeenCalled();
}); });
it('skips restore when no entry stored', () => { it('skips restore when no entry stored', () => {
setDocHeight(5000); setDocHeight(5000);
restoreScroll('popstate'); restoreScroll('popstate', url('/wishlist'));
expect(scrollToSpy).not.toHaveBeenCalled(); expect(scrollToSpy).not.toHaveBeenCalled();
}); });
it('skips restore for trivial scrollY (noise)', () => { it('skips restore for trivial scrollY (noise)', () => {
sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 10 })); sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 10 }));
setDocHeight(5000); setDocHeight(5000);
restoreScroll('popstate'); restoreScroll('popstate', url('/wishlist'));
expect(scrollToSpy).not.toHaveBeenCalled(); expect(scrollToSpy).not.toHaveBeenCalled();
}); });
it('scrolls immediately when document is already tall enough', async () => { it('scrolls immediately when document is already tall enough', async () => {
sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 })); sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1000 }));
setDocHeight(5000); setDocHeight(5000);
restoreScroll('popstate'); restoreScroll('popstate', url('/wishlist'));
// rAF resolves on next microtask in jsdom
await new Promise((r) => requestAnimationFrame(() => r(null))); await new Promise((r) => requestAnimationFrame(() => r(null)));
expect(scrollToSpy).toHaveBeenCalledWith({ top: 1000, left: 0, behavior: 'instant' }); expect(scrollToSpy).toHaveBeenCalledWith({ top: 1000, left: 0, behavior: 'instant' });
}); });
it('waits via rAF until document grows tall enough', async () => { it('waits via rAF until document grows tall enough', async () => {
sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1500 })); sessionStorage.setItem(STORAGE_KEY, JSON.stringify({ '/wishlist': 1500 }));
// Initially short — would clamp.
setDocHeight(900); setDocHeight(900);
restoreScroll('popstate'); restoreScroll('popstate', url('/wishlist'));
await new Promise((r) => requestAnimationFrame(() => r(null))); await new Promise((r) => requestAnimationFrame(() => r(null)));
expect(scrollToSpy).not.toHaveBeenCalled(); expect(scrollToSpy).not.toHaveBeenCalled();
// Simulate async data loading and document growing.
setDocHeight(3000); setDocHeight(3000);
// Allow rAF to fire again.
await new Promise((r) => requestAnimationFrame(() => r(null))); await new Promise((r) => requestAnimationFrame(() => r(null)));
expect(scrollToSpy).toHaveBeenCalledWith({ top: 1500, left: 0, behavior: 'instant' }); expect(scrollToSpy).toHaveBeenCalledWith({ top: 1500, left: 0, behavior: 'instant' });
}); });