From 3325ab5decbe352b68ac1c9d9d30a7675cf57529 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 04:39:53 +0000 Subject: [PATCH 1/7] fix(web): write useUrlState changes with window.history so a reload keeps them useUrlState's setter wrote the URL with router.replace, which commits the address bar only once the route's server render returns. On PR #718's preview that render took 4.3 s, and a reload inside that window brought the Explorer back to Newest (aw-verify F-1). The setter now writes with window.history.replaceState (pushState in push mode, deduped like router.push). Next.js syncs both into useSearchParams without a server render, so the URL holds the choice as soon as setState returns. The write builds from the live location, so two keys set in one tick both land. The clean-up effects keep using the router. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../web/src/lib/hooks/useDebouncedUrlState.ts | 8 +- .../web/src/lib/hooks/useUrlState.spec.ts | 140 +++++++++++++++++- packages/web/src/lib/hooks/useUrlState.ts | 90 ++++++++--- 3 files changed, 207 insertions(+), 31 deletions(-) diff --git a/packages/web/src/lib/hooks/useDebouncedUrlState.ts b/packages/web/src/lib/hooks/useDebouncedUrlState.ts index ec49961cb..223d74c98 100644 --- a/packages/web/src/lib/hooks/useDebouncedUrlState.ts +++ b/packages/web/src/lib/hooks/useDebouncedUrlState.ts @@ -9,11 +9,11 @@ * ## Why this exists * `useUrlState` writes the URL synchronously on every `setState` call. For a * value that changes rapidly — a text search box, a range slider — that means a - * `router.replace` per keystroke/drag. Each navigation re-renders the route - * subtree (every component subscribed to `useSearchParams`), which makes the - * control feel laggy and floods the URL/history with intermediate values. + * URL write per keystroke/drag. Each write re-renders every component + * subscribed to `useSearchParams`, which makes the control feel laggy and + * floods the URL with intermediate values (and the history, in `push` mode). * Wrapping the write in `startTransition` only lowers its priority; it does not - * stop the per-keystroke navigation. + * stop the per-keystroke write. * * This hook decouples the two concerns: * 1. **Responsiveness** — the returned value is backed by local `useState`, so diff --git a/packages/web/src/lib/hooks/useUrlState.spec.ts b/packages/web/src/lib/hooks/useUrlState.spec.ts index d5f1ac9f5..75cec70cf 100644 --- a/packages/web/src/lib/hooks/useUrlState.spec.ts +++ b/packages/web/src/lib/hooks/useUrlState.spec.ts @@ -12,6 +12,9 @@ * - toAbsolute path normalisation * - buildUrl: param write, delete on default, preserve unrelated params * - isPathnameAllowed: exact match, prefix match, non-match, array overload + * - writeParamToHistory: the setter's URL write — replaceState vs pushState, + * built from the live location, synchronous (so a reload right after a + * choice keeps it), against a fake `window` * * The optimistic-update contract is documented in the comments below as a * spec for future integration tests, and the SSR contract is documented via @@ -25,6 +28,8 @@ import { toAbsolute, buildUrl, isPathnameAllowed, + writeParamToHistory, + type HistoryWindow, } from './useUrlState'; // ── serialise ───────────────────────────────────────────────────────────────── @@ -202,6 +207,134 @@ describe('isPathnameAllowed', () => { }); }); +// ── writeParamToHistory ─────────────────────────────────────────────────────── + +const ORIGIN = 'https://lorekit.test'; + +/** + * A fake `window` whose history calls move its location the way a browser's + * do, and record every call, so a test reads "the address bar" right after a + * write without a DOM. + */ +function fakeWindow(start: string) { + const calls: { method: 'pushState' | 'replaceState'; url: string }[] = []; + const location = { pathname: '/', search: '' }; + const go = (url: string) => { + const u = new URL(url, ORIGIN); + location.pathname = u.pathname; + location.search = u.search; + }; + go(start); + const history = { + pushState: (_data: unknown, _unused: string, url?: string | URL | null) => { + calls.push({ method: 'pushState', url: String(url) }); + go(String(url)); + }, + replaceState: (_data: unknown, _unused: string, url?: string | URL | null) => { + calls.push({ method: 'replaceState', url: String(url) }); + go(String(url)); + }, + }; + const win: HistoryWindow = { location, history }; + /** What a reload would load right now. */ + const addressBar = () => `${location.pathname}${location.search}`; + return { win, calls, addressBar }; +} + +/** What `useUrlState` reads back for `key` after loading `url` (a reload). */ +function readBack(url: string, key: string, defaultValue: T): T { + return deserialise(new URL(url, ORIGIN).searchParams.get(key), defaultValue); +} + +describe('writeParamToHistory', () => { + describe('replace mode', () => { + it('writes the new URL with replaceState and never pushState', () => { + const { win, calls } = fakeWindow('/lore'); + const url = writeParamToHistory(win, 'status', 'archived', 'active', 'replace'); + expect(url).toBe('/lore?status=%22archived%22'); + expect(calls).toEqual([{ method: 'replaceState', url: '/lore?status=%22archived%22' }]); + }); + + it('changes the address bar before it returns — no navigation to wait for', () => { + const { win, addressBar } = fakeWindow('/lore'); + writeParamToHistory(win, 'status', 'archived', 'active', 'replace'); + expect(addressBar()).toBe('/lore?status=%22archived%22'); + }); + }); + + describe('Recent read on the Explorer (aw-verify F-1)', () => { + it('choosing Recent read writes sort=%22last_opened_at%22, and a reload reads it back', () => { + const { win, calls, addressBar } = fakeWindow('/lore'); + writeParamToHistory(win, 'sort', 'last_opened_at', null, 'replace'); + expect(calls).toEqual([{ method: 'replaceState', url: '/lore?sort=%22last_opened_at%22' }]); + // The reload happens immediately: the address bar already holds the choice. + expect(addressBar()).toBe('/lore?sort=%22last_opened_at%22'); + expect(readBack(addressBar(), 'sort', null)).toBe('last_opened_at'); + }); + + it('choosing Newest (the default) again drops sort, and a reload reads the default', () => { + const { win, addressBar } = fakeWindow('/lore?sort=%22last_opened_at%22'); + writeParamToHistory(win, 'sort', null, null, 'replace'); + expect(addressBar()).toBe('/lore'); + expect(readBack(addressBar(), 'sort', null)).toBe(null); + }); + }); + + describe('push mode', () => { + it('adds a history entry with pushState when the URL changes', () => { + const { win, calls } = fakeWindow('/lore'); + writeParamToHistory(win, 'lesson', { scope: 'global', key: 'k' }, null, 'push'); + expect(calls).toHaveLength(1); + expect(calls[0].method).toBe('pushState'); + expect(readBack(calls[0].url, 'lesson', null)).toEqual({ scope: 'global', key: 'k' }); + }); + + it('replaces instead of pushing when the value is unchanged, leaving no empty Back step', () => { + const { win, calls } = fakeWindow('/lore?lesson=%7B%22scope%22%3A%22global%22%2C%22key%22%3A%22k%22%7D'); + writeParamToHistory(win, 'lesson', { scope: 'global', key: 'k' }, null, 'push'); + expect(calls.map((c) => c.method)).toEqual(['replaceState']); + }); + + it('pushes when clearing a set value back to its default', () => { + const { win, calls, addressBar } = fakeWindow('/lore?lesson=%22x%22'); + writeParamToHistory(win, 'lesson', null, null, 'push'); + expect(calls.map((c) => c.method)).toEqual(['pushState']); + expect(addressBar()).toBe('/lore'); + }); + }); + + describe('builds from the live URL', () => { + it('keeps params already in the address bar', () => { + const { win, addressBar } = fakeWindow('/lore?scope=%22repo%3A%3Aa%2Fb%22&q=%22auth%22'); + writeParamToHistory(win, 'sort', 'opened_count', null, 'replace'); + const qs = new URL(addressBar(), ORIGIN).searchParams; + expect(qs.get('scope')).toBe('"repo::a/b"'); + expect(qs.get('q')).toBe('"auth"'); + expect(qs.get('sort')).toBe('"opened_count"'); + }); + + it('keeps a param written earlier in the same tick (two keys set by one handler)', () => { + const { win, addressBar } = fakeWindow('/lore'); + writeParamToHistory(win, 'status', 'archived', 'active', 'replace'); + writeParamToHistory(win, 'sort', 'last_opened_at', null, 'replace'); + expect(readBack(addressBar(), 'status', 'active')).toBe('archived'); + expect(readBack(addressBar(), 'sort', null)).toBe('last_opened_at'); + }); + + it('removes the param when set to its default and keeps the rest', () => { + const { win, addressBar } = fakeWindow('/lore?status=%22archived%22&sort=%22opened_count%22'); + writeParamToHistory(win, 'status', 'active', 'active', 'replace'); + expect(addressBar()).toBe('/lore?sort=%22opened_count%22'); + }); + + it('writes on the page the browser shows, keeping its other params (e.g. a Storybook iframe)', () => { + const { win, addressBar } = fakeWindow('/iframe.html?id=pages-lore--default&viewMode=story'); + writeParamToHistory(win, 'sort', 'last_opened_at', null, 'replace'); + expect(addressBar()).toBe('/iframe.html?id=pages-lore--default&viewMode=story&sort=%22last_opened_at%22'); + }); + }); +}); + /** * ── SSR contract (documented, not executed here) ────────────────────────────── * @@ -226,9 +359,10 @@ describe('isPathnameAllowed', () => { * ── Optimistic update contract (documented, not executed here) ───────────────── * * 1. User calls setState(next). - * 2. setOptimisticValue(next) fires synchronously → re-render shows `next` immediately. - * 3. router.replace(url) fires asynchronously → navigation starts. - * 4. When navigation completes, searchParams changes → urlValue becomes `next`. + * 2. writeOptimistic(next) fires synchronously → re-render shows `next` immediately. + * 3. writeParamToHistory writes the address bar synchronously (tested above); + * Next.js applies it to useSearchParams() in a transition. + * 4. On that render, searchParams changes → urlValue becomes `next`. * 5. useEffect detects serialise(urlValue) === serialise(optimisticValue) → sets * optimisticValue back to UNSET (URL is now the source of truth again). * 6. External URL changes (back button, direct link) update searchParams → urlValue diff --git a/packages/web/src/lib/hooks/useUrlState.ts b/packages/web/src/lib/hooks/useUrlState.ts index b9d056e39..2001f405e 100644 --- a/packages/web/src/lib/hooks/useUrlState.ts +++ b/packages/web/src/lib/hooks/useUrlState.ts @@ -12,18 +12,26 @@ * Next.js renders a shell on the server and fills in the real URL-derived value * on the client, avoiding hydration mismatches. * + * ## Writing the URL + * `setState` writes the address bar with `window.history.replaceState` (or + * `pushState` in `push` mode) — see {@link writeParamToHistory}. Next.js syncs + * those calls into `useSearchParams()` / `usePathname()` without a server + * render, so the URL holds the new value the moment `setState` returns, and a + * reload right after a choice restores it. (It used to go through + * `router.replace`, which commits the URL only once the route's server render + * returns — seconds on a slow backend, long enough for a reload to lose it.) + * * ## Optimistic updates - * URL navigation via `router.replace` is asynchronous: the new search params do - * not appear in `useSearchParams()` until the router has completed the navigation - * cycle (a React transition). Without optimistic state, users would see the old - * value for tens of milliseconds between clicking and the URL updating — causing + * The address bar changes synchronously, but Next.js applies the change to + * `useSearchParams()` inside a React transition, a render later. Without + * optimistic state, readers would show the old value for that render — causing * perceived lag and flicker. * * This hook solves it with a local `optimistic` state that is updated - * synchronously on every `setState` call, while the URL write happens in the - * background. When the URL finally reflects the new value (next render after - * navigation), `optimistic` is reset to `UNSET` so the URL is once again the - * source of truth. + * synchronously on every `setState` call, alongside the URL write. When + * `useSearchParams()` reflects the new value (the next render), + * `optimistic` is reset to `UNSET` so the URL is once again the source of + * truth. * * The optimistic value is shared by KEY across every mounted instance * (`url-state-optimistic.ts`), not held per instance: a write from one @@ -76,8 +84,9 @@ export interface UseUrlStateOptions { */ cleanOnPathname?: string | string[]; /** - * Navigation method. Use 'push' only when you need the back-button to undo - * state changes. 'replace' (default) keeps history clean. + * How a write enters the browser history. Use 'push' only when you need the + * back-button to undo state changes (`history.pushState`). 'replace' + * (default, `history.replaceState`) keeps history clean. * @default 'replace' */ navigationMode?: 'push' | 'replace'; @@ -127,6 +136,48 @@ export function buildUrl( return `${pathname}${qs ? `?${qs}` : ''}`; } +/** The parts of `window` a URL write touches — passed in so the write is testable without a DOM. */ +export interface HistoryWindow { + location: Pick; + history: Pick; +} + +/** + * Write one param to the address bar, synchronously, and return the URL written. + * + * Goes through `window.history`, not the router: Next.js patches `pushState` / + * `replaceState` so `useSearchParams()` and `usePathname()` follow the change, + * without the server render a `router.replace` waits on before it commits the + * URL. That wait is what let a reload restore the previous value. + * + * Builds from the LIVE location, not a render's `useSearchParams()` snapshot, + * so two writes in one tick (two keys set by one handler) both land: the second + * starts from the URL the first just wrote. It also keeps the write on the page + * the browser is actually showing. + * + * `push` adds a history entry only when this param's value changes, the same + * dedupe the App Router applies to `router.push`, so re-setting the current + * value leaves no empty Back step. + */ +export function writeParamToHistory( + win: HistoryWindow, + key: string, + next: T, + defaultValue: T, + mode: 'push' | 'replace', +): string { + const params = new URLSearchParams(win.location.search); + const nextRaw = serialise(next) === serialise(defaultValue) ? null : serialise(next); + const changed = params.get(key) !== nextRaw; + const url = buildUrl(win.location.pathname, params, key, next, defaultValue); + if (mode === 'push' && changed) { + win.history.pushState(null, '', url); + } else { + win.history.replaceState(null, '', url); + } + return url; +} + /** * Whether `pathname` matches any of the `allowed` paths (exact or prefix match). * All inputs are normalised to start with '/'. @@ -187,14 +238,6 @@ export function useUrlState( // catching up; falls back to the true URL value otherwise. const value = optimisticValue !== UNSET ? optimisticValue : urlValue; - // ── Navigate ref ─────────────────────────────────────────────────────────── - // Updated synchronously during render (no useEffect) so there is never a - // stale-closure window where setState uses the wrong navigate function. - const navigateRef = useRef['push']>( - navigationMode === 'push' ? router.push : router.replace, - ); - navigateRef.current = navigationMode === 'push' ? router.push : router.replace; - // ── setState ─────────────────────────────────────────────────────────────── const setState = useCallback>( (valueOrUpdater) => { @@ -207,16 +250,15 @@ export function useUrlState( ? (valueOrUpdater as (prev: T) => T)(prev) : valueOrUpdater; - // 1. Optimistic: immediate UI update in every instance reading `key`, no - // wait for the navigation round-trip. + // 1. Optimistic: immediate UI update in every instance reading `key`, + // before `useSearchParams()` catches up a render later. writeOptimistic(key, next, pathname); - // 2. Persist: update the URL asynchronously via the router. - const url = buildUrl(pathname, searchParams, key, next, defaultValue); - navigateRef.current(url, { scroll: false }); + // 2. Persist: write the address bar now, so a reload keeps the choice. + writeParamToHistory(window, key, next, defaultValue, navigationMode); }, // eslint-disable-next-line react-hooks/exhaustive-deps - [searchParams, pathname, key, defaultValue], + [searchParams, pathname, key, defaultValue, navigationMode], ); // ── cleanOnPathname effect ───────────────────────────────────────────────── From 8755cdf88d73d5d035f2c83dc7d8c80a82b4f350 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 04:42:31 +0000 Subject: [PATCH 2/7] test(web): a sort choice is in the address bar the moment it is made Interaction story on the Explorer: choosing Recent read must put sort=%22last_opened_at%22 in window.location in the same step, and Newest must remove it. Fails against the router.replace setter (sort reads null), passes with the window.history write. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../lore/LorePage.test.stories.tsx | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx b/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx index e1a3647d9..432611d22 100644 --- a/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx +++ b/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx @@ -1,5 +1,5 @@ import type { Meta, StoryObj } from '@storybook/react'; -import { expect, within } from 'storybook/test'; +import { expect, userEvent, within } from 'storybook/test'; import LorePageClient from './LorePageClient'; import { memoryHandlers, FROZEN_NOW } from '@/mocks/memories'; @@ -65,3 +65,34 @@ export const RendersScopeSelector: Story = { }, }; +/** The page's own `?sort=` value, as a reload would read it. */ +const sortInAddressBar = () => new URLSearchParams(window.location.search).get('sort'); + +/** + * A sort choice is in the address bar the moment it is made, so a reload right + * after it keeps it. It used to reach the URL only after a server render + * returned — 4.3 s on PR #718's preview, where a reload in between restored + * Newest. Storybook's App Router mock never feeds `window.history` back into + * `useSearchParams`, so this reads the browser's own location: what a reload + * would load. + */ +export const SortChoiceIsInTheAddressBarAtOnce: Story = { + play: async ({ canvasElement, step }) => { + const canvas = within(canvasElement); + // Exact name: the phone-width control is labelled "Sort memories: ". + const sort = await canvas.findByRole('combobox', { name: 'Sort memories' }); + + await step('Choosing Recent read writes sort to the URL in the same step', async () => { + await userEvent.selectOptions(sort, 'Recent read'); + await expect(sortInAddressBar()).toBe('"last_opened_at"'); + await expect(sort).toHaveValue('last_opened_at'); + }); + + await step('Choosing Newest, the default, removes sort again', async () => { + await userEvent.selectOptions(sort, 'Newest'); + await expect(sortInAddressBar()).toBeNull(); + await expect(sort).toHaveValue('created_at'); + }); + }, +}; + From f114c0edf54157fa3ddd9caeeb2a849d09ceb11a Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 04:54:09 +0000 Subject: [PATCH 3/7] fix(web): one Back step for a handler that sets two push-mode keys FilterBar's Clear all sets filters and then retention, both push-mode, in one click. Writing each with pushState left two Back steps, the first landing on a state the reader never saw. Within one task only the first URL-changing push-mode write pushes; later ones replace that entry, as the router used to collapse same-tick navigations. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../web/src/lib/hooks/useUrlState.spec.ts | 17 +++++++++++++ packages/web/src/lib/hooks/useUrlState.ts | 24 +++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/web/src/lib/hooks/useUrlState.spec.ts b/packages/web/src/lib/hooks/useUrlState.spec.ts index 75cec70cf..8684e725e 100644 --- a/packages/web/src/lib/hooks/useUrlState.spec.ts +++ b/packages/web/src/lib/hooks/useUrlState.spec.ts @@ -301,6 +301,23 @@ describe('writeParamToHistory', () => { expect(calls.map((c) => c.method)).toEqual(['pushState']); expect(addressBar()).toBe('/lore'); }); + + it('one handler that sets two keys adds one history entry (FilterBar "Clear all")', () => { + const { win, calls, addressBar } = fakeWindow('/lore?filters=%5B1%5D&retention=%7B%22a%22%3A1%7D'); + // Same task, as in one click handler: filters, then retention. + writeParamToHistory(win, 'filters', [], [], 'push'); + writeParamToHistory(win, 'retention', {}, {}, 'push'); + expect(calls.map((c) => c.method)).toEqual(['pushState', 'replaceState']); + expect(addressBar()).toBe('/lore'); + }); + + it('separate clicks each add their own history entry', async () => { + const { win, calls } = fakeWindow('/lore'); + writeParamToHistory(win, 'lesson', '"a"', null, 'push'); + await Promise.resolve(); // the click handler returned; the next click is a new task + writeParamToHistory(win, 'lesson', '"b"', null, 'push'); + expect(calls.map((c) => c.method)).toEqual(['pushState', 'pushState']); + }); }); describe('builds from the live URL', () => { diff --git a/packages/web/src/lib/hooks/useUrlState.ts b/packages/web/src/lib/hooks/useUrlState.ts index 2001f405e..a5a6f6ba2 100644 --- a/packages/web/src/lib/hooks/useUrlState.ts +++ b/packages/web/src/lib/hooks/useUrlState.ts @@ -136,6 +136,24 @@ export function buildUrl( return `${pathname}${qs ? `?${qs}` : ''}`; } +/** + * Whether a push-mode write already added a history entry in the current task. + * One handler that sets several push-mode keys is one user step — FilterBar's + * "Clear all" sets `filters` and then `retention` — so it gets one Back step: + * the first write that changes the URL pushes, and the rest in the same task + * replace that entry. Cleared on the next microtask, once the handler returns. + * (The router collapsed same-tick navigations the same way.) + */ +let pushedThisTask = false; + +function notePush(): void { + if (pushedThisTask) return; + pushedThisTask = true; + queueMicrotask(() => { + pushedThisTask = false; + }); +} + /** The parts of `window` a URL write touches — passed in so the write is testable without a DOM. */ export interface HistoryWindow { location: Pick; @@ -157,7 +175,8 @@ export interface HistoryWindow { * * `push` adds a history entry only when this param's value changes, the same * dedupe the App Router applies to `router.push`, so re-setting the current - * value leaves no empty Back step. + * value leaves no empty Back step — and only once per task (`pushedThisTask`), + * so one click that sets two keys is one Back step. */ export function writeParamToHistory( win: HistoryWindow, @@ -170,8 +189,9 @@ export function writeParamToHistory( const nextRaw = serialise(next) === serialise(defaultValue) ? null : serialise(next); const changed = params.get(key) !== nextRaw; const url = buildUrl(win.location.pathname, params, key, next, defaultValue); - if (mode === 'push' && changed) { + if (mode === 'push' && changed && !pushedThisTask) { win.history.pushState(null, '', url); + notePush(); } else { win.history.replaceState(null, '', url); } From 23348b2a1c6344e71786bcf1ad73300d478c29db Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 04:55:44 +0000 Subject: [PATCH 4/7] =?UTF-8?q?chore(web):=20simplify=20pass=20=E2=80=94?= =?UTF-8?q?=20one-line=20docblocks,=20WHY=20comments=20at=20the=20lines=20?= =?UTF-8?q?they=20explain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../lore/LorePage.test.stories.tsx | 11 ++--- .../web/src/lib/hooks/useUrlState.spec.ts | 2 +- packages/web/src/lib/hooks/useUrlState.ts | 43 ++++++------------- 3 files changed, 17 insertions(+), 39 deletions(-) diff --git a/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx b/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx index 432611d22..52b5bb0e4 100644 --- a/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx +++ b/packages/web/src/app/(dashboard)/lore/LorePage.test.stories.tsx @@ -66,16 +66,11 @@ export const RendersScopeSelector: Story = { }; /** The page's own `?sort=` value, as a reload would read it. */ +// The browser's location, not useSearchParams(): Storybook's App Router mock +// never feeds `window.history` back into it. const sortInAddressBar = () => new URLSearchParams(window.location.search).get('sort'); -/** - * A sort choice is in the address bar the moment it is made, so a reload right - * after it keeps it. It used to reach the URL only after a server render - * returned — 4.3 s on PR #718's preview, where a reload in between restored - * Newest. Storybook's App Router mock never feeds `window.history` back into - * `useSearchParams`, so this reads the browser's own location: what a reload - * would load. - */ +/** A sort choice is in the address bar the moment it is made, so a reload right after it keeps it. */ export const SortChoiceIsInTheAddressBarAtOnce: Story = { play: async ({ canvasElement, step }) => { const canvas = within(canvasElement); diff --git a/packages/web/src/lib/hooks/useUrlState.spec.ts b/packages/web/src/lib/hooks/useUrlState.spec.ts index 8684e725e..a7eda4a0a 100644 --- a/packages/web/src/lib/hooks/useUrlState.spec.ts +++ b/packages/web/src/lib/hooks/useUrlState.spec.ts @@ -262,7 +262,7 @@ describe('writeParamToHistory', () => { }); }); - describe('Recent read on the Explorer (aw-verify F-1)', () => { + describe('Recent read on the Explorer', () => { it('choosing Recent read writes sort=%22last_opened_at%22, and a reload reads it back', () => { const { win, calls, addressBar } = fakeWindow('/lore'); writeParamToHistory(win, 'sort', 'last_opened_at', null, 'replace'); diff --git a/packages/web/src/lib/hooks/useUrlState.ts b/packages/web/src/lib/hooks/useUrlState.ts index a5a6f6ba2..d2b0aed6e 100644 --- a/packages/web/src/lib/hooks/useUrlState.ts +++ b/packages/web/src/lib/hooks/useUrlState.ts @@ -13,13 +13,9 @@ * on the client, avoiding hydration mismatches. * * ## Writing the URL - * `setState` writes the address bar with `window.history.replaceState` (or - * `pushState` in `push` mode) — see {@link writeParamToHistory}. Next.js syncs - * those calls into `useSearchParams()` / `usePathname()` without a server - * render, so the URL holds the new value the moment `setState` returns, and a - * reload right after a choice restores it. (It used to go through - * `router.replace`, which commits the URL only once the route's server render - * returns — seconds on a slow backend, long enough for a reload to lose it.) + * `setState` writes the address bar with `window.history` ({@link writeParamToHistory}), + * so it holds the new value as soon as `setState` returns and a reload keeps it. + * A `router.replace` committed the URL only after a server render — seconds on a slow backend. * * ## Optimistic updates * The address bar changes synchronously, but Next.js applies the change to @@ -136,14 +132,9 @@ export function buildUrl( return `${pathname}${qs ? `?${qs}` : ''}`; } -/** - * Whether a push-mode write already added a history entry in the current task. - * One handler that sets several push-mode keys is one user step — FilterBar's - * "Clear all" sets `filters` and then `retention` — so it gets one Back step: - * the first write that changes the URL pushes, and the rest in the same task - * replace that entry. Cleared on the next microtask, once the handler returns. - * (The router collapsed same-tick navigations the same way.) - */ +// One handler that sets several push-mode keys (FilterBar's "Clear all") is one +// user step, so only its first URL-changing write pushes; the rest replace that +// entry. Cleared on the next microtask, once the handler has returned. let pushedThisTask = false; function notePush(): void { @@ -162,21 +153,8 @@ export interface HistoryWindow { /** * Write one param to the address bar, synchronously, and return the URL written. - * - * Goes through `window.history`, not the router: Next.js patches `pushState` / - * `replaceState` so `useSearchParams()` and `usePathname()` follow the change, - * without the server render a `router.replace` waits on before it commits the - * URL. That wait is what let a reload restore the previous value. - * - * Builds from the LIVE location, not a render's `useSearchParams()` snapshot, - * so two writes in one tick (two keys set by one handler) both land: the second - * starts from the URL the first just wrote. It also keeps the write on the page - * the browser is actually showing. - * - * `push` adds a history entry only when this param's value changes, the same - * dedupe the App Router applies to `router.push`, so re-setting the current - * value leaves no empty Back step — and only once per task (`pushedThisTask`), - * so one click that sets two keys is one Back step. + * @param mode - `push` adds a Back step when the value changes, once per task; `replace` never does. + * @returns The URL now in the address bar. */ export function writeParamToHistory( win: HistoryWindow, @@ -185,10 +163,15 @@ export function writeParamToHistory( defaultValue: T, mode: 'push' | 'replace', ): string { + // The LIVE location, not a render's useSearchParams() snapshot: a second + // write in the same tick then starts from the URL the first just wrote. const params = new URLSearchParams(win.location.search); const nextRaw = serialise(next) === serialise(defaultValue) ? null : serialise(next); + // Re-setting the current value pushes nothing, as router.push dedupes it. const changed = params.get(key) !== nextRaw; const url = buildUrl(win.location.pathname, params, key, next, defaultValue); + // window.history, not the router: Next.js syncs it into useSearchParams() + // without the server render a router.replace waits on before the URL commits. if (mode === 'push' && changed && !pushedThisTask) { win.history.pushState(null, '', url); notePush(); From 18456b562c886361e9b92bf6143c0691d73be748 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 05:08:54 +0000 Subject: [PATCH 5/7] docs(web): URL-state comments no longer describe router.replace as the write path url-state-optimistic's header and the two MemorySidebarProvider guards explained a router round trip and a same-tick clobber that the window.history write from the live URL no longer has. The guards stay; their reasons now read in the past tense. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- .../providers/MemorySidebarProvider.tsx | 16 ++++++++-------- .../web/src/lib/hooks/url-state-optimistic.ts | 16 ++++++++-------- packages/web/src/lib/open-lesson.ts | 10 +++++----- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/web/src/components/providers/MemorySidebarProvider.tsx b/packages/web/src/components/providers/MemorySidebarProvider.tsx index b24433c34..043c31c35 100644 --- a/packages/web/src/components/providers/MemorySidebarProvider.tsx +++ b/packages/web/src/components/providers/MemorySidebarProvider.tsx @@ -203,8 +203,8 @@ export function MemorySidebarProvider({ children }: MemorySidebarProviderProps) // than reimplemented for the alias. const urlMemoryId = searchParams.get('memoryId') ?? lessonParamMemoryId; // Closing the sheet dismisses the id locally rather than rewriting the URL — - // the pure `activeMemoryId` docblock has the why (a second router.replace in - // the same tick clobbers the Explorer's scope/filter write). + // the pure `activeMemoryId` docblock has the why (when URL writes went through + // the router, a second one in the same tick clobbered the Explorer's write). const [dismissedMemoryId, setDismissedMemoryId] = useState(null); const memoryId = activeMemoryId(urlMemoryId, dismissedMemoryId); const { data: memoryByIdLesson } = useMemoryById(memoryId); @@ -258,12 +258,12 @@ export function MemorySidebarProvider({ children }: MemorySidebarProviderProps) [setLessonRef], ); - // Only write the URL when a lesson is actually open. Calling this - // unconditionally (e.g. from LoreExplorer's scope-select handler, which - // closes any open lesson while also setting ?scope) would fire a second - // router.replace built from the *pre-scope* search params, clobbering the - // scope param that was set in the same tick. Guarding on lessonRef makes the - // close a no-op navigation when nothing is open. + // Only write the URL when a lesson is actually open. LoreExplorer's + // scope-select handler closes any open lesson while also setting ?scope; when + // URL writes went through the router, that second write was built from the + // *pre-scope* params and clobbered the scope. Writes now start from the live + // URL, and guarding on lessonRef still keeps the close a no-op when nothing + // is open. const closeLesson = useCallback((rawReason?: MemoryCloseReason) => { // Only a close that closed SOMETHING is an event. `LoreExplorer` calls this // on every filter/scope/status change whether or not a memory is open, so diff --git a/packages/web/src/lib/hooks/url-state-optimistic.ts b/packages/web/src/lib/hooks/url-state-optimistic.ts index 648e7c918..9d3159a60 100644 --- a/packages/web/src/lib/hooks/url-state-optimistic.ts +++ b/packages/web/src/lib/hooks/url-state-optimistic.ts @@ -1,13 +1,13 @@ /** * The optimistic layer behind {@link useUrlState}, shared by URL param KEY. * - * A URL write through the router lands a navigation later: the new search - * params reach `useSearchParams()` only once the router has finished its cycle, - * which for a dynamic page includes a server round trip. `useUrlState` hides - * that by showing the written value immediately. That layer used to be local - * `useState` per hook INSTANCE, so it only hid the wait from the component that - * wrote — every other component reading the same param kept the old value - * until the navigation landed. Two components reading `?filters=` (the + * A URL write reaches `useSearchParams()` a render after it is made: Next.js + * applies a `window.history` write inside a transition (and a router write, + * before `useUrlState` used `window.history`, only after a server round trip). + * `useUrlState` hides that by showing the written value immediately. That layer + * used to be local `useState` per hook INSTANCE, so it only hid the wait from + * the component that wrote — every other component reading the same param kept + * the old value until the write landed. Two components reading `?filters=` (the * Explorer and the detail panel applying a filter into it) disagreed for that * whole window, and the Explorer looked slow to react to the panel. * @@ -17,7 +17,7 @@ * node vitest project. * * An entry is dropped — the URL becomes the source of truth again — when: - * - the URL now holds the written value (the navigation landed); + * - the URL now holds the written value (the write landed); * - the pathname differs from the one it was written on (the reader left the * page the write was aimed at, so it can never land there); * - the last instance reading the key unmounts (nothing is left to show it, diff --git a/packages/web/src/lib/open-lesson.ts b/packages/web/src/lib/open-lesson.ts index 13557b2c4..e7ab15ef9 100644 --- a/packages/web/src/lib/open-lesson.ts +++ b/packages/web/src/lib/open-lesson.ts @@ -103,11 +103,11 @@ export function resolveLessonParam(value: unknown, raw: string | null): LessonPa * The `memoryId` that is still open: the URL's param, unless the user has * dismissed that exact id. * - * Closing the sheet deliberately does NOT strip `?memoryId=` from the URL. That - * strip needed its own `router.replace`, built from the pre-mutation search - * params, and the Lore Explorer calls `closeLesson()` in the same tick as its - * own scope / filter writes — so the second navigation landed last and clobbered - * the scope or filter the user had just set. Dismissal is local state instead, + * Closing the sheet deliberately does NOT strip `?memoryId=` from the URL. When + * this was decided, that strip needed its own `router.replace`, built from the + * pre-mutation search params, and the Lore Explorer calls `closeLesson()` in the + * same tick as its own scope / filter writes — so the second navigation landed + * last and clobbered the scope or filter the user had just set. Dismissal is local state instead, * keyed BY id so a later deep link to a different memory still opens the sheet. * The cost is that the param survives in the URL, which only means a refresh * re-opens the deep link it names. From 4dbcb4abf7aea6109df3dfe9f4dd4725279e9914 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 05:08:55 +0000 Subject: [PATCH 6/7] fix(web): defer useUrlState's history write by a microtask Next.js installs its window.history patch in an App Router effect. On a hard load a child's mount effect in the same commit runs first, so a setter called there (OrganizationManager clearing a stale ?org=) would hit the raw History API and Next's router would never see the write. React runs a commit's mount effects in one synchronous flush, so one microtask later the patch is in; still before any paint or reload. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/web/src/lib/hooks/useUrlState.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/web/src/lib/hooks/useUrlState.ts b/packages/web/src/lib/hooks/useUrlState.ts index d2b0aed6e..d38519b3d 100644 --- a/packages/web/src/lib/hooks/useUrlState.ts +++ b/packages/web/src/lib/hooks/useUrlState.ts @@ -13,8 +13,8 @@ * on the client, avoiding hydration mismatches. * * ## Writing the URL - * `setState` writes the address bar with `window.history` ({@link writeParamToHistory}), - * so it holds the new value as soon as `setState` returns and a reload keeps it. + * `setState` writes the address bar with `window.history` ({@link writeParamToHistory}) + * a microtask after it returns — before any paint — so a reload right after a choice keeps it. * A `router.replace` committed the URL only after a server render — seconds on a slow backend. * * ## Optimistic updates @@ -257,8 +257,9 @@ export function useUrlState( // before `useSearchParams()` catches up a render later. writeOptimistic(key, next, pathname); - // 2. Persist: write the address bar now, so a reload keeps the choice. - writeParamToHistory(window, key, next, defaultValue, navigationMode); + // 2. Persist to the address bar, a microtask later: on a hard load Next.js + // patches window.history in an effect that runs after child mount effects. + queueMicrotask(() => writeParamToHistory(window, key, next, defaultValue, navigationMode)); }, // eslint-disable-next-line react-hooks/exhaustive-deps [searchParams, pathname, key, defaultValue, navigationMode], From f5b20e1498ebb6f3e78737c306511845a0cc6d51 Mon Sep 17 00:00:00 2001 From: "mads.thines" Date: Thu, 22 Oct 2026 05:16:59 +0000 Subject: [PATCH 7/7] docs(web): the setter's URL write is a microtask later, not synchronous Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com> --- packages/web/src/lib/hooks/useDebouncedUrlState.ts | 2 +- packages/web/src/lib/hooks/useUrlState.spec.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/web/src/lib/hooks/useDebouncedUrlState.ts b/packages/web/src/lib/hooks/useDebouncedUrlState.ts index 223d74c98..7a34c4048 100644 --- a/packages/web/src/lib/hooks/useDebouncedUrlState.ts +++ b/packages/web/src/lib/hooks/useDebouncedUrlState.ts @@ -7,7 +7,7 @@ * debounced, while the value it returns stays instantly responsive. * * ## Why this exists - * `useUrlState` writes the URL synchronously on every `setState` call. For a + * `useUrlState` writes the URL on every `setState` call (a microtask later). For a * value that changes rapidly — a text search box, a range slider — that means a * URL write per keystroke/drag. Each write re-renders every component * subscribed to `useSearchParams`, which makes the control feel laggy and diff --git a/packages/web/src/lib/hooks/useUrlState.spec.ts b/packages/web/src/lib/hooks/useUrlState.spec.ts index a7eda4a0a..be149ec26 100644 --- a/packages/web/src/lib/hooks/useUrlState.spec.ts +++ b/packages/web/src/lib/hooks/useUrlState.spec.ts @@ -377,8 +377,8 @@ describe('writeParamToHistory', () => { * * 1. User calls setState(next). * 2. writeOptimistic(next) fires synchronously → re-render shows `next` immediately. - * 3. writeParamToHistory writes the address bar synchronously (tested above); - * Next.js applies it to useSearchParams() in a transition. + * 3. A microtask later, writeParamToHistory writes the address bar (tested + * above); Next.js applies it to useSearchParams() in a transition. * 4. On that render, searchParams changes → urlValue becomes `next`. * 5. useEffect detects serialise(urlValue) === serialise(optimisticValue) → sets * optimisticValue back to UNSET (URL is now the source of truth again).