Skip to content
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -65,3 +65,29 @@ 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. */
export const SortChoiceIsInTheAddressBarAtOnce: Story = {
play: async ({ canvasElement, step }) => {
const canvas = within(canvasElement);
// Exact name: the phone-width control is labelled "Sort memories: <order>".
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');
});
},
};

Original file line number Diff line number Diff line change
Expand Up @@ -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<string | null>(null);
const memoryId = activeMemoryId(urlMemoryId, dismissedMemoryId);
const { data: memoryByIdLesson } = useMemoryById(memoryId);
Expand Down Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions packages/web/src/lib/hooks/url-state-optimistic.ts
Original file line number Diff line number Diff line change
@@ -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.
*
Expand All @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions packages/web/src/lib/hooks/useDebouncedUrlState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
* 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
* `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
Expand Down
157 changes: 154 additions & 3 deletions packages/web/src/lib/hooks/useUrlState.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,6 +28,8 @@ import {
toAbsolute,
buildUrl,
isPathnameAllowed,
writeParamToHistory,
type HistoryWindow,
} from './useUrlState';

// ── serialise ─────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -202,6 +207,151 @@ 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<T>(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', () => {
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');
});

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', () => {
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) ──────────────────────────────
*
Expand All @@ -226,9 +376,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. 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).
* 6. External URL changes (back button, direct link) update searchParams → urlValue
Expand Down
Loading
Loading