diff --git a/e2e/timeline.spec.ts b/e2e/timeline.spec.ts index 1b8e911c0..4a3818413 100644 --- a/e2e/timeline.spec.ts +++ b/e2e/timeline.spec.ts @@ -332,6 +332,11 @@ test("value lines: quarterly means, gaps kept, short ones bridged, as many as th ); expect(dots).toHaveLength(15); await page.keyboard.press("Escape"); + // The menu closes on an exit animation and only then hands focus back to + // its trigger, which scrolls the page to the toolbar. Measured before + // that, the scroll to the last row below was undone a moment later. + await expect(options).toHaveCount(0); + await expect(trigger).toBeFocused(); await expect .poll(() => log.timelineQueries.at(-1)?.get("values")?.split(",").length) .toBe(15); diff --git a/src/__tests__/baselines/source-file-lines.json b/src/__tests__/baselines/source-file-lines.json index 8eaa83771..96b76c3ce 100644 --- a/src/__tests__/baselines/source-file-lines.json +++ b/src/__tests__/baselines/source-file-lines.json @@ -1,7 +1,7 @@ { "$comment": "Production files over 800 lines and their length. Maintained by src/__tests__/source-size-ratchet.test.ts.", "count": 123, - "total": 145465, + "total": 145428, "entries": { "app/api/dashboard/summary/route.ts": 1032, "app/api/documents/inbound/route.ts": 811, @@ -18,7 +18,7 @@ "components/charts/mood-chart.tsx": 1166, "components/cycle/log-day-sheet.tsx": 1639, "components/documents/document-detail-sheet.tsx": 1304, - "components/documents/documents-view.tsx": 1244, + "components/documents/documents-view.tsx": 1223, "components/insights/coach-panel/chat-bubble.tsx": 888, "components/insights/coach-panel/coach-conversation.tsx": 1286, "components/insights/coach-panel/use-coach.ts": 1486, @@ -29,7 +29,7 @@ "components/measurement-reminders/vorsorge-section.tsx": 1536, "components/measurements/measurement-form.tsx": 815, "components/measurements/measurement-list.tsx": 2019, - "components/medications/detail/medication-detail-tabs.tsx": 1176, + "components/medications/detail/medication-detail-tabs.tsx": 1160, "components/medications/dose-history-ledger.tsx": 1052, "components/medications/medication-table.tsx": 853, "components/medications/wizard/wizard-payload.ts": 1414, diff --git a/src/__tests__/baselines/source-function-lines.json b/src/__tests__/baselines/source-function-lines.json index 175d5af26..ca5b8cee3 100644 --- a/src/__tests__/baselines/source-function-lines.json +++ b/src/__tests__/baselines/source-function-lines.json @@ -1,7 +1,7 @@ { "$comment": "Production functions over 200 lines, keyed \"file :: name\", and their length. Maintained by src/__tests__/source-size-ratchet.test.ts.", "count": 411, - "total": 148137, + "total": 148109, "entries": { "app/achievements/page.tsx :: AchievementsPage": 206, "app/api/admin/backups/upload/route.ts :: apiHandler(arg)": 314, @@ -74,7 +74,7 @@ "app/insights/medications/page.tsx :: InsightsMedikamentePage": 265, "app/insights/page-client.tsx :: InsightsPageClient": 376, "app/labs/page.tsx :: LabsPage": 228, - "app/medications/page-client.tsx :: MedicationsPageClient": 591, + "app/medications/page-client.tsx :: MedicationsPageClient": 590, "app/notifications/page.tsx :: NotificationsPage": 327, "app/page-client.tsx :: DashboardPageClient": 2131, "app/page-client.tsx :: DashboardPageClient > (anonymous)": 1279, @@ -118,7 +118,7 @@ "components/daily/today-hero.tsx :: TodayHero": 488, "components/dashboard/dashboard-header.tsx :: DashboardHeader": 215, "components/dashboard/medication-intake-quick-add.tsx :: MedicationIntakeQuickAdd": 384, - "components/day/day-layer.tsx :: DayLayer": 360, + "components/day/day-layer.tsx :: DayLayer": 358, "components/day/day-view.tsx :: DayView": 309, "components/documents/document-ai-section.tsx :: DocumentAiSection": 262, "components/documents/document-bulk-bar.tsx :: DocumentBulkBar": 204, @@ -135,7 +135,7 @@ "components/illness/new-episode-sheet.tsx :: NewEpisodeSheet": 264, "components/insights/coach-panel/attachment-picker.tsx :: AttachmentPicker": 208, "components/insights/coach-panel/chat-bubble.tsx :: ChatBubbleImpl": 419, - "components/insights/coach-panel/coach-conversation.tsx :: CoachConversation": 1012, + "components/insights/coach-panel/coach-conversation.tsx :: CoachConversation": 1011, "components/insights/coach-panel/coach-input.tsx :: CoachInput": 259, "components/insights/coach-panel/coach-model-picker.tsx :: CoachModelPicker": 212, "components/insights/coach-panel/conversations-panel.tsx :: ConversationsPanel": 317, @@ -186,7 +186,7 @@ "components/measurements/measurement-list.tsx :: MeasurementList > data.measurements.map(arg)": 223, "components/measurements/measurement-list.tsx :: MeasurementList > data.measurements.map(arg) #2": 229, "components/medications/detail/efficacy/efficacy-chart.tsx :: EfficacyChart": 361, - "components/medications/detail/medication-detail-tabs.tsx :: MedicationDetailTabs": 787, + "components/medications/detail/medication-detail-tabs.tsx :: MedicationDetailTabs": 771, "components/medications/dose-history-add-dialog.tsx :: LedgerAddDialog": 223, "components/medications/dose-history-ledger.tsx :: DoseHistoryLedger": 404, "components/medications/dose-history-ledger.tsx :: LedgerRowItem": 314, @@ -233,7 +233,7 @@ "components/settings/access/managed-profile-edit-form.tsx :: ManagedProfileEditFields": 208, "components/settings/access/managed-profile-guardians.tsx :: ManagedProfileGuardians": 234, "components/settings/account-section/index.tsx :: AccountSection": 659, - "components/settings/ai/codex-provider-form.tsx :: CodexProviderForm": 281, + "components/settings/ai/codex-provider-form.tsx :: CodexProviderForm": 280, "components/settings/ai/fallback-chain-card.tsx :: FallbackChainCard": 228, "components/settings/ai/runtime-actions-row.tsx :: RuntimeActionsRow": 282, "components/settings/api-section.tsx :: ApiTokensCard": 366, @@ -248,7 +248,7 @@ "components/settings/import-panel/json-import-card.tsx :: JsonImportCard": 223, "components/settings/import-panel/record-csv-import-card.tsx :: RecordCsvImportSheet": 460, "components/settings/insights-pill-order-section.tsx :: InsightsPillOrderSection": 255, - "components/settings/integrations/connections-panel.tsx :: ConnectionsPanel": 236, + "components/settings/integrations/connections-panel.tsx :: ConnectionsPanel": 230, "components/settings/integrations/document-sources-card.tsx :: SourceForm": 306, "components/settings/integrations/google-health-card.tsx :: GoogleHealthCard": 300, "components/settings/integrations/integration-card.tsx :: IntegrationCard": 209, @@ -276,7 +276,7 @@ "components/timeline/series-point-tips.tsx :: SeriesPointTips": 212, "components/timeline/timeline-chart.tsx :: ChartBody": 243, "components/timeline/timeline-chronicle.tsx :: TimelineChronicle": 274, - "components/timeline/timeline-view.tsx :: TimelineView": 502, + "components/timeline/timeline-view.tsx :: TimelineView": 501, "components/ui/responsive-sheet.tsx :: ResponsiveSheet": 239, "components/vaccinations/catalog-picker.tsx :: CatalogPicker": 221, "components/vaccinations/custom-vaccine-sheet.tsx :: CustomVaccineSheet": 246, diff --git a/src/__tests__/helpers/url-state-browser.ts b/src/__tests__/helpers/url-state-browser.ts new file mode 100644 index 000000000..989a1d8cd --- /dev/null +++ b/src/__tests__/helpers/url-state-browser.ts @@ -0,0 +1,92 @@ +/** + * A stand-in browser for code that writes through `@/lib/url-state`: a + * location, a session history that records every write, and the event + * plumbing the helper dispatches on. The router "adopts" each write on the + * next microtask, as Next does a render later, so a test that awaits between + * writes sees them land one after another. + */ +import { vi } from "vitest"; + +import { reportRouterSearch, resetUrlStateForTests } from "@/lib/url-state"; + +export interface UrlStateBrowser { + /** `pushState` hrefs, in order. */ + push: ReturnType; + /** `replaceState` hrefs, in order. */ + replace: ReturnType; + current: () => string; + uninstall: () => void; +} + +export function installUrlStateBrowser( + initialUrl: string, + { autoAdopt = true }: { autoAdopt?: boolean } = {}, +): UrlStateBrowser { + resetUrlStateForTests(); + const push = vi.fn(); + const replace = vi.fn(); + let url = initialUrl; + let state: unknown = null; + const parsed = () => new URL(url, "http://localhost"); + const location = { + get pathname() { + return parsed().pathname; + }, + get search() { + return parsed().search; + }, + get hash() { + return parsed().hash; + }, + get href() { + return parsed().href; + }, + }; + const adopt = () => { + if (autoAdopt) queueMicrotask(() => reportRouterSearch(location.search)); + }; + const history = { + get state() { + return state; + }, + pushState(next: unknown, _unused: string, href: string) { + url = href; + state = next; + push(href); + adopt(); + }, + replaceState(next: unknown, _unused: string, href: string) { + url = href; + state = next; + replace(href); + adopt(); + }, + back() {}, + }; + const g = globalThis as unknown as Record; + const previous = g.window; + g.window = { + location, + history, + addEventListener() {}, + removeEventListener() {}, + dispatchEvent() { + return true; + }, + requestAnimationFrame(callback: (time: number) => void) { + callback(0); + return 0; + }, + cancelAnimationFrame() {}, + }; + return { + push, + replace, + current: () => url, + uninstall: () => { + resetUrlStateForTests(); + if (previous === undefined) delete g.window; + else g.window = previous; + }, + }; +} diff --git a/src/__tests__/url-state-writes-guard.test.ts b/src/__tests__/url-state-writes-guard.test.ts new file mode 100644 index 000000000..8c0740162 --- /dev/null +++ b/src/__tests__/url-state-writes-guard.test.ts @@ -0,0 +1,73 @@ +/** + * Every write of page state into the query string goes through + * `@/lib/url-state`, which derives it from the live address and holds it + * until the router has the previous one. + * + * Next 16.4 adopts a native history write a render later, and a write + * computed from `useSearchParams` starts from the URL before the last write. + * Each surface that wrote on its own met one of the two: a quick second + * step of the day undone, a closed document sheet left open, cleared vault + * filters coming back as `?encounter=`. A raw `history.pushState` / + * `replaceState`, or a router write built from the rendered search params, + * reopens the class, so both are refused outside the helper and the few + * places listed here with their reason. + */ +import { describe, expect, it } from "vitest"; + +import { readSourceFolder } from "@/__tests__/helpers/source-files"; + +const RAW_HISTORY_WRITE = /\bhistory\s*\.\s*(?:pushState|replaceState)\s*\(/; +const RENDERED_SEARCH = /\bsearchParams\??\.toString\(\)/; +const ROUTER_WRITE = /\brouter\s*\.\s*(?:push|replace)\s*\(/; + +const ALLOWED: Record = { + "lib/url-state.ts": "the helper itself", + "components/clinician/share-unlock-gate.tsx": + "scrubs a passphrase from the fragment synchronously, before the unlock request leaves; no query state", + "components/documents/documents-view.tsx": + "closeDocumentSelectionHistoryEntry takes a history-shaped argument for its tests; the page passes `urlStateHistory`", +}; + +describe("query-string writes go through @/lib/url-state", () => { + const files = readSourceFolder("src", { + floor: 500, + include: (rel) => + !rel.startsWith("generated/") && + !rel.includes("__tests__/") && + !/\.test\.tsx?$/.test(rel), + }); + + it("reads the whole tree (floor)", () => { + expect(files.length).toBeGreaterThan(1000); + }); + + it("finds no raw history write outside the helper", () => { + const offenders = files + .filter(([rel, text]) => !ALLOWED[rel] && RAW_HISTORY_WRITE.test(text)) + .map(([rel]) => rel); + expect(offenders).toEqual([]); + }); + + it("finds no router write built from the rendered search params", () => { + const offenders = files + .filter( + ([rel, text]) => + !ALLOWED[rel] && + RENDERED_SEARCH.test(text) && + ROUTER_WRITE.test(text), + ) + .map(([rel]) => rel); + expect(offenders).toEqual([]); + }); + + it("still sees the helper's own writes (the matcher is alive)", () => { + const helper = files.find(([rel]) => rel === "lib/url-state.ts"); + expect(helper && RAW_HISTORY_WRITE.test(helper[1])).toBe(true); + for (const rel of Object.keys(ALLOWED)) { + expect( + files.some(([path]) => path === rel), + `${rel} is still in the tree`, + ).toBe(true); + } + }); +}); diff --git a/src/app/coach/page-client.tsx b/src/app/coach/page-client.tsx index 8c08143a6..a985e967b 100644 --- a/src/app/coach/page-client.tsx +++ b/src/app/coach/page-client.tsx @@ -16,6 +16,7 @@ import { queryKeys } from "@/lib/query-keys"; import { apiGet } from "@/lib/api/api-fetch"; import { COACH_SCOPE_SOURCES } from "@/lib/ai/coach/scope-sources"; import type { CoachScopeSource } from "@/lib/ai/coach/types"; +import { writeUrlState } from "@/lib/url-state"; /** * v1.12.0 (Coach v2 #6) — full-page Coach conversation. @@ -133,13 +134,7 @@ function CoachPageBody() { ); useEffect(() => { if (!openSettingsOnData) return; - const url = new URL(window.location.href); - url.searchParams.delete("settings"); - window.history.replaceState( - window.history.state, - "", - url.pathname + url.search, - ); + writeUrlState((params) => params.delete("settings"), { state: "keep" }); }, [openSettingsOnData]); // #781 — a navigation away mid-reply recorded the interrupted conversation diff --git a/src/app/measurements/page.tsx b/src/app/measurements/page.tsx index ce8a70cd8..c45cc89ad 100644 --- a/src/app/measurements/page.tsx +++ b/src/app/measurements/page.tsx @@ -23,6 +23,7 @@ import { Plus } from "lucide-react"; import { useRouter, useSearchParams } from "next/navigation"; import { useTranslations } from "@/lib/i18n/context"; import { SUB_PAGE_SLUGS } from "@/lib/insights/sub-page-metric"; +import { dropParams, writeUrlState } from "@/lib/url-state"; const INSIGHT_METRIC_RETURN_PATHS: Readonly> = Object.fromEntries( @@ -120,12 +121,10 @@ export default function MeasurementsPage() { // during SSR. useEffect(() => { if (!addParam && !returnToParam) return; - const next = new URLSearchParams(searchParams.toString()); - next.delete("add"); - next.delete("returnTo"); - const search = next.toString(); - router.replace(search ? `/measurements?${search}` : "/measurements"); - }, [addParam, returnToParam, searchParams, router]); + // From the live query at the moment of the write (`@/lib/url-state`), so + // a filter written meanwhile is never put back to its old value. + writeUrlState(dropParams("add", "returnTo"), { afterPaint: true }); + }, [addParam, returnToParam]); useEffect(() => { if (!isLoading && !isAuthenticated) { diff --git a/src/app/medications/page-client.tsx b/src/app/medications/page-client.tsx index 5de387002..e197120d3 100644 --- a/src/app/medications/page-client.tsx +++ b/src/app/medications/page-client.tsx @@ -60,6 +60,7 @@ import { useMedicationComplianceSummaryAll } from "@/lib/queries/use-medication- import { useMedicationListLayout } from "@/lib/queries/use-medication-list-layout"; import { applyMedicationOrder } from "@/lib/medications/medication-order"; import { DEFAULT_TIMEZONE } from "@/lib/tz/format"; +import { setAllParams, writeUrlState } from "@/lib/url-state"; interface Schedule { id: string; @@ -310,14 +311,13 @@ export default function MedicationsPageClient() { if (!shouldOpenFromUrl) return; // Child effects may run before the App Router installs its native-history - // integration, while the responsive sheet also settles its first client - // layout. Wait until the following painted frame so replaceState preserves - // Next's internals and synchronizes useSearchParams without starting a - // router navigation that can remount the open wizard and discard fields. + // integration while the sheet settles its first layout. Wait two painted + // frames so the write keeps Next's internals and syncs useSearchParams + // without a router navigation that would remount the open wizard. let cleanupFrame: number | undefined; const layoutFrame = window.requestAnimationFrame(() => { cleanupFrame = window.requestAnimationFrame(() => { - window.history.replaceState(null, "", "/medications"); + writeUrlState(setAllParams("")); }); }); diff --git a/src/app/mood/page-client.tsx b/src/app/mood/page-client.tsx index 33f62f98c..0139f4f56 100644 --- a/src/app/mood/page-client.tsx +++ b/src/app/mood/page-client.tsx @@ -19,6 +19,7 @@ import { useRouter, useSearchParams } from "next/navigation"; import { useEffect } from "react"; import { useTranslations } from "@/lib/i18n/context"; import { InlineAction } from "@/components/ui/inline-action"; +import { writeUrlState } from "@/lib/url-state"; export default function MoodPageClient() { const { user, isAuthenticated, isLoading } = useAuth(); @@ -99,11 +100,10 @@ export default function MoodPageClient() { useEffect(() => { if (!searchParams.has("add")) return; - const next = new URLSearchParams(searchParams.toString()); - next.delete("add"); - const search = next.toString(); - router.replace(search ? `/mood?${search}` : "/mood"); - }, [searchParams, router]); + // From the live query at the moment of the write, never the rendered + // copy, so a filter written meanwhile is kept (`@/lib/url-state`). + writeUrlState((params) => params.delete("add"), { afterPaint: true }); + }, [searchParams]); useEffect(() => { if (isLoading) return; diff --git a/src/components/day/__tests__/day-layer-controller.test.ts b/src/components/day/__tests__/day-layer-controller.test.ts index dcfaaa23c..ae2b071c4 100644 --- a/src/components/day/__tests__/day-layer-controller.test.ts +++ b/src/components/day/__tests__/day-layer-controller.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { reportRouterSearch, resetUrlStateForTests } from "@/lib/url-state"; + /** * The history contract of the day layer, against a small stand-in for the * browser's session history: @@ -27,6 +29,10 @@ function installFakeBrowser(initialUrl: string) { get search() { return new URL(entries[index]!.url, "http://x").search; }, + hash: "", + get href() { + return new URL(entries[index]!.url, "http://x").href; + }, }; const history = { get state() { @@ -48,11 +54,31 @@ function installFakeBrowser(initialUrl: string) { }, }; const g = globalThis as unknown as Record; - g.window = { location, history }; + g.window = { + location, + history, + addEventListener() {}, + removeEventListener() {}, + dispatchEvent() { + return true; + }, + }; g.document = { activeElement: null, body: {} }; + // Next adopts each write a render later; the URL-state queue holds the + // next write until it has. Reading the address bar stands for the router + // catching up with every write made so far. + const catchUp = () => { + for (let i = 0; i < 10; i++) reportRouterSearch(location.search); + }; return { - current: () => entries[index]!.url, - length: () => entries.length, + current: () => { + catchUp(); + return entries[index]!.url; + }, + length: () => { + catchUp(); + return entries.length; + }, }; } @@ -61,6 +87,7 @@ let browser: ReturnType; const TODAY = "2026-01-10"; beforeEach(() => { + resetUrlStateForTests(); browser = installFakeBrowser("/insights/blood-pressure?range=90"); }); diff --git a/src/components/day/day-layer-controller.ts b/src/components/day/day-layer-controller.ts index 6ce4eb5b7..d30379acc 100644 --- a/src/components/day/day-layer-controller.ts +++ b/src/components/day/day-layer-controller.ts @@ -3,13 +3,13 @@ import { useSyncExternalStore } from "react"; import { DAY_QUERY_PARAM, type DateKey } from "@/lib/day/contract"; +import { currentUrlSearch, writeUrlState } from "@/lib/url-state"; import { dayHistoryState, historyEntryOwnedByLayer, parseDayParam, shiftDateKey, - withDayHref, } from "./day-url"; /** @@ -134,10 +134,6 @@ export interface OpenDayOptions { trigger?: HTMLElement | null; } -function currentLocation() { - return { pathname: window.location.pathname, search: window.location.search }; -} - /** Open `date` over the current page. */ export function openDay(date: DateKey, options: OpenDayOptions = {}): void { if (typeof window === "undefined") return; @@ -154,13 +150,10 @@ export function openDay(date: DateKey, options: OpenDayOptions = {}): void { ? memory.trigger.getBoundingClientRect().top : null; emit(); - const { pathname, search } = currentLocation(); - const href = withDayHref(pathname, search, date); - if (historyEntryOwnedByLayer(window.history.state)) { - window.history.replaceState(dayHistoryState(date), "", href); - } else { - window.history.pushState(dayHistoryState(date), "", href); - } + writeUrlState((params) => params.set(DAY_QUERY_PARAM, date), { + mode: historyEntryOwnedByLayer(window.history.state) ? "replace" : "push", + state: dayHistoryState(date), + }); } /** @@ -191,7 +184,8 @@ let pendingStep: { delta: number; today: DateKey } | null = null; export function stepDay(delta: number, today: DateKey): DateKey | null { if (typeof window === "undefined") return null; const from = parseDayParam( - new URLSearchParams(window.location.search).get(DAY_QUERY_PARAM), + // The live address, a queued write included (`@/lib/url-state`). + new URLSearchParams(currentUrlSearch()).get(DAY_QUERY_PARAM), today, ); if (from === null) { @@ -218,15 +212,12 @@ export function stepDay(delta: number, today: DateKey): DateKey | null { export function jumpDay(next: DateKey): void { memory.focus = null; emit(); - const { pathname, search } = currentLocation(); const owned = historyEntryOwnedByLayer(window.history.state); - window.history.replaceState( + writeUrlState((params) => params.set(DAY_QUERY_PARAM, next), { // A deep link stays a deep link: marking it as pushed would make a later // close go Back off the page the person arrived on. - owned ? dayHistoryState(next) : null, - "", - withDayHref(pathname, search, next), - ); + state: owned ? dayHistoryState(next) : null, + }); } /** @@ -283,6 +274,5 @@ export function closeDay(): void { window.history.back(); return; } - const { pathname, search } = currentLocation(); - window.history.replaceState(null, "", withDayHref(pathname, search, null)); + writeUrlState((params) => params.delete(DAY_QUERY_PARAM)); } diff --git a/src/components/day/day-layer.tsx b/src/components/day/day-layer.tsx index cf6a94b36..f74872099 100644 --- a/src/components/day/day-layer.tsx +++ b/src/components/day/day-layer.tsx @@ -2,6 +2,8 @@ import dynamic from "next/dynamic"; import { usePathname, useSearchParams } from "next/navigation"; + +import { reportRouterSearch, writeUrlState } from "@/lib/url-state"; import { Suspense, useCallback, @@ -56,12 +58,7 @@ import { takeDayYield, useDayFocus, } from "./day-layer-controller"; -import { - parseDayParam, - shiftDateKey, - stripDayOf, - withDayHref, -} from "./day-url"; +import { parseDayParam, shiftDateKey, stripDayOf } from "./day-url"; import { useLongDayLabel } from "./day-label"; import type { DayViewProps } from "./day-view"; import { usePrefetchDay } from "./use-day"; @@ -167,12 +164,14 @@ function DayLayer() { // A parameter the layer cannot open leaves the URL quietly. useEffect(() => { if (raw === null || date !== null) return; - window.history.replaceState( - null, - "", - withDayHref(window.location.pathname, window.location.search, null), - ); + writeUrlState((params) => params.delete(DAY_QUERY_PARAM)); }, [raw, date]); + // The layer reads the router's copy of the URL; it tells the URL-state + // queue when that copy has caught up with a write. + const routerSearch = searchParams.toString(); + useEffect(() => { + reportRouterSearch(routerSearch); + }, [routerSearch]); useEffect(() => { publishOpenDay(date); @@ -282,11 +281,7 @@ function DayLayer() { // Read again, not from the render: a Back on this page has just closed it. if (!readDayOpen(owner)) return; restoring.current = true; - window.history.replaceState( - null, - "", - withDayHref(window.location.pathname, window.location.search, railDay), - ); + writeUrlState((params) => params.set(DAY_QUERY_PARAM, railDay)); }, [shell, raw, leftOpen, railDay, owner]); // What the polite region says: the day that just opened or was stepped to. diff --git a/src/components/documents/documents-view.tsx b/src/components/documents/documents-view.tsx index ffbb2e535..12b6614ee 100644 --- a/src/components/documents/documents-view.tsx +++ b/src/components/documents/documents-view.tsx @@ -30,15 +30,8 @@ import { Wrench, } from "lucide-react"; import Link from "next/link"; -import { usePathname, useRouter, useSearchParams } from "next/navigation"; -import { - useCallback, - useEffect, - useMemo, - useRef, - useState, - useSyncExternalStore, -} from "react"; +import { usePathname, useRouter } from "next/navigation"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; import { showErrorToast } from "@/lib/errors/error-toast"; @@ -53,6 +46,12 @@ import { useAuth } from "@/hooks/use-auth"; import { DEFAULT_TIMEZONE } from "@/lib/tz/format"; import { usePullToRefresh } from "@/hooks/use-pull-to-refresh"; import { useUrlFilterSync } from "@/hooks/use-url-filter-sync"; +import { + currentUrlSearch, + urlStateHistory, + useUrlSearch, + writeUrlState, +} from "@/lib/url-state"; import { PullToRefreshIndicator } from "@/components/ui/pull-to-refresh-indicator"; import { apiGet, apiPost } from "@/lib/api/api-fetch"; import { @@ -117,26 +116,6 @@ const BULK_VISIT_OPTIONS = 12; const DOCUMENT_QUERY_ID_PATTERN = /^[a-zA-Z0-9_-]{1,40}(?![\s\S])/; const DOCUMENT_SELECTION_HISTORY_KEY = "__healthlogPushedDocumentSelection"; -/** - * The query string in the address bar, read at render time. - * - * The open sheet follows `?doc=`, and `useSearchParams` is not a faithful - * copy of it: Next 16.4 applies the copy of a `pushState` later than the write, - * and a Back pressed in between moves the address bar from `?doc=` to the - * page without it while `useSearchParams` can read "no document" on both sides. - * The sheet then saw no change and stayed open. `window.location` is never - * behind; `popstate` re-reads it on every traversal, and every other render - * (including the ones Next's own navigations cause) reads it afresh. - */ -function subscribeToTraversal(onChange: () => void): () => void { - window.addEventListener("popstate", onChange); - return () => window.removeEventListener("popstate", onChange); -} - -function readLocationSearch(): string { - return window.location.search; -} - export function documentSelectionHistoryState( documentId: string, ): Record { @@ -287,7 +266,6 @@ export function DocumentsView() { const canManageDocuments = canManageDomain("documents"); const router = useRouter(); const pathname = usePathname(); - const searchParams = useSearchParams(); const queryClient = useQueryClient(); const coachLaunch = useCoachLaunch(); @@ -326,8 +304,12 @@ export function DocumentsView() { const handle = setTimeout(() => { const trimmed = searchDraft.trim(); if (trimmed === (filters.q ?? "")) return; + // Live filters, not the rendered ones a "Clear filters" just removed. applyFilters( - { ...filters, q: trimmed === "" ? undefined : trimmed }, + (current) => + trimmed === (current.q ?? "") + ? current + : { ...current, q: trimmed === "" ? undefined : trimmed }, "replace", ); }, 200); @@ -356,46 +338,46 @@ export function DocumentsView() { return () => window.removeEventListener("keydown", onKeyDown); }, []); + // Every facet toggles against the live filters (`applyFilters` updater). const toggleKind = (kind: InboundDocumentKindValue) => { - const current = new Set(filters.kinds ?? []); - if (current.has(kind)) { - current.delete(kind); - } else { - current.add(kind); - } - applyFilters( - { - ...filters, - kinds: current.size > 0 ? [...current].sort() : undefined, - }, - "push", - ); + applyFilters((current) => { + const kinds = new Set(current.kinds ?? []); + if (kinds.has(kind)) kinds.delete(kind); + else kinds.add(kind); + return { + ...current, + kinds: kinds.size > 0 ? [...kinds].sort() : undefined, + }; + }, "push"); }; const toggleEpisode = (episodeId: string) => { applyFilters( - { - ...filters, - episodeId: filters.episodeId === episodeId ? undefined : episodeId, - }, + (current) => ({ + ...current, + episodeId: current.episodeId === episodeId ? undefined : episodeId, + }), "push", ); }; const toggleEncounter = (encounterId: string) => { applyFilters( - { - ...filters, + (current) => ({ + ...current, encounterId: - filters.encounterId === encounterId ? undefined : encounterId, - }, + current.encounterId === encounterId ? undefined : encounterId, + }), "push", ); }; const toggleYear = (year: number) => { applyFilters( - { ...filters, year: filters.year === year ? undefined : year }, + (current) => ({ + ...current, + year: current.year === year ? undefined : year, + }), "push", ); }; @@ -845,13 +827,11 @@ export function DocumentsView() { const [detailOpen, setDetailOpen] = useState(false); const handedOffDocumentRef = useRef(null); - // Before hydration the server's view of the query stands in for the URL. - const locationSearch = useSyncExternalStore( - subscribeToTraversal, - readLocationSearch, - () => searchParams.toString(), - ); - const rawDocParam = new URLSearchParams(locationSearch).get("doc"); + // `?doc=` read live (`@/lib/url-state`): Next 16.4 adopts a pushState a + // render after the write, and a Back pressed in between left the router's + // copy reading "no document" on both sides, so the sheet stayed open. + const liveParams = useUrlSearch(); + const rawDocParam = liveParams.get("doc"); const docParam = rawDocParam !== null && DOCUMENT_QUERY_ID_PATTERN.test(rawDocParam) ? rawDocParam @@ -870,12 +850,15 @@ export function DocumentsView() { const openDetail = useCallback( (id: string) => { - const href = documentSelectionHref(pathname, window.location.search, id); + const href = documentSelectionHref(pathname, currentUrlSearch(), id); if (href === null) return; handedOffDocumentRef.current = null; setDetailId(id); setDetailOpen(true); - window.history.pushState(documentSelectionHistoryState(id), "", href); + writeUrlState((params) => params.set("doc", id), { + mode: "push", + state: documentSelectionHistoryState(id), + }); }, [pathname], ); @@ -912,9 +895,9 @@ export function DocumentsView() { ); handedOffDocumentRef.current = closeDocumentSelectionAfterCoachHandoff( - window.history, + urlStateHistory, pathname, - window.location.search, + currentUrlSearch(), docParam, handedOffDocumentRef.current, coachLaunch?.closeIntent, @@ -940,14 +923,10 @@ export function DocumentsView() { // the moment the sheet re-opens or the parameter changes — which is what // makes re-opening within the window safe. const verify = window.setTimeout(() => { - if (!documentSelectionSurvivedClose(window.location.search, docParam)) { + if (!documentSelectionSurvivedClose(currentUrlSearch(), docParam)) { return; } - window.history.replaceState( - null, - "", - withoutDocumentSelectionHref(pathname, window.location.search), - ); + writeUrlState((params) => params.delete("doc")); }, CLOSE_TRAVERSAL_GRACE_MS); return () => window.clearTimeout(verify); }, [ diff --git a/src/components/insights/coach-panel/coach-conversation.tsx b/src/components/insights/coach-panel/coach-conversation.tsx index 429c23c9b..79fa0bd9e 100644 --- a/src/components/insights/coach-panel/coach-conversation.tsx +++ b/src/components/insights/coach-panel/coach-conversation.tsx @@ -68,6 +68,7 @@ import { useSendCoachMessage, } from "./use-coach"; import { translateOrNull } from "@/lib/i18n/translate-code"; +import { currentUrlSearch, writeUrlHref } from "@/lib/url-state"; /** * v1.12.0 (Coach v2 #6) — shared chat surface for both the Coach drawer @@ -371,9 +372,8 @@ export function CoachConversation({ // a shared tab) lands on it. The drawer has no URL of its own. function syncPageUrl(conversationId: string | null) { if (surface !== "page" || typeof window === "undefined") return; - const href = coachPageHref(conversationId, window.location.search); - if (window.location.pathname + window.location.search === href) return; - window.history.replaceState(window.history.state, "", href); + const href = coachPageHref(conversationId, currentUrlSearch()); + writeUrlHref(href, { state: "keep" }); } const send = useSendCoachMessage({ diff --git a/src/components/measurements/__tests__/measurement-list-url-sync.test.tsx b/src/components/measurements/__tests__/measurement-list-url-sync.test.tsx index 6f7545643..c6225b9b0 100644 --- a/src/components/measurements/__tests__/measurement-list-url-sync.test.tsx +++ b/src/components/measurements/__tests__/measurement-list-url-sync.test.tsx @@ -15,7 +15,7 @@ * user's click (post-render setState is a no-op under the server renderer; * the router spies record the navigation the click causes). */ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; const baseMeasurements = [ @@ -113,6 +113,13 @@ vi.mock("@/components/ui/filter-bar", () => ({ import { I18nProvider } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { MeasurementList } from "../measurement-list"; +import { + installUrlStateBrowser, + type UrlStateBrowser, +} from "@/__tests__/helpers/url-state-browser"; + +let browser: UrlStateBrowser = installUrlStateBrowser("/"); +afterEach(() => browser.uninstall()); function render(search: string) { mockSearch = search; @@ -121,11 +128,17 @@ function render(search: string) { dateRange = null; valueRange = null; recordedQueryKeys.length = 0; - return renderToStaticMarkup( + const html = renderToStaticMarkup( , ); + // The live address the writes start from (`@/lib/url-state`). + browser.uninstall(); + browser = installUrlStateBrowser( + search ? `/measurements?${search}` : "/measurements", + ); + return html; } const typeSelect = () => selects[0]!; @@ -140,40 +153,37 @@ describe("MeasurementList — filter changes write the URL", () => { it("pushes the type facet", () => { render(""); typeSelect().onValueChange("WEIGHT"); - expect(pushMock).toHaveBeenCalledWith("/measurements?type=WEIGHT", { - scroll: false, - }); + expect(browser.push).toHaveBeenCalledWith("/measurements?type=WEIGHT"); }); it("pushes the source facet on top of the active type", () => { render("type=WEIGHT"); sourceSelect().onValueChange("WITHINGS"); - expect(pushMock).toHaveBeenCalledWith( + expect(browser.push).toHaveBeenCalledWith( "/measurements?type=WEIGHT&source=WITHINGS", - { scroll: false }, ); }); it("replaces (no history entry) for a committed date bound", () => { render(""); dateRange!.onFromChange!("2026-05-01"); - expect(replaceMock).toHaveBeenCalledWith("/measurements?from=2026-05-01", { - scroll: false, - }); - expect(pushMock).not.toHaveBeenCalled(); + expect(browser.replace).toHaveBeenCalledWith( + "/measurements?from=2026-05-01", + ); + expect(browser.push).not.toHaveBeenCalled(); }); it("clearing the last facet restores the bare pathname", () => { render("type=WEIGHT"); typeSelect().onValueChange("ALL"); - expect(pushMock).toHaveBeenCalledWith("/measurements", { scroll: false }); + expect(browser.push).toHaveBeenCalledWith("/measurements"); }); it("reset clears every facet in one push", () => { render("type=WEIGHT&source=WITHINGS&from=2026-05-01&min=60"); expect(barProps!.isFiltered).toBe(true); barProps!.onReset(); - expect(pushMock).toHaveBeenCalledWith("/measurements", { scroll: false }); + expect(browser.push).toHaveBeenCalledWith("/measurements"); }); }); diff --git a/src/components/measurements/measurement-list.tsx b/src/components/measurements/measurement-list.tsx index 207b30e81..6ebe3eeec 100644 --- a/src/components/measurements/measurement-list.tsx +++ b/src/components/measurements/measurement-list.tsx @@ -411,7 +411,7 @@ export function MeasurementList({ // v1.8.5 — when `lockedType` is set the list is pinned to that metric, the // type selector is hidden, and the URL belongs to the insights subpage — // the remaining filters stay in component memory (`syncToUrl: false`). - const { filters, applyFilters } = useUrlFilterSync({ + const { filters, applyFilters, patchFilters } = useUrlFilterSync({ parse: parseMeasurementListSearchParams, serialise: measurementListFiltersToSearch, syncToUrl: lockedType === undefined, @@ -505,25 +505,25 @@ export function MeasurementList({ // vault's chips); the committed date bounds replace — a from/to pair set in // one sitting should not cost two Back presses. const setTypeFilter = (value: string) => { - applyFilters({ ...filters, type: value === "ALL" ? undefined : value }); + patchFilters({ type: value === "ALL" ? undefined : value }); setPage(1); clearSelection(); }; const setSourceFilter = (value: string) => { - applyFilters({ ...filters, source: value === "ALL" ? undefined : value }); + patchFilters({ source: value === "ALL" ? undefined : value }); setPage(1); clearSelection(); }; const setFromDay = (value: string) => { - applyFilters({ ...filters, fromDay: value || undefined }, "replace"); + patchFilters({ fromDay: value || undefined }, "replace"); setPage(1); clearSelection(); }; const setToDay = (value: string) => { - applyFilters({ ...filters, toDay: value || undefined }, "replace"); + patchFilters({ toDay: value || undefined }, "replace"); setPage(1); clearSelection(); }; @@ -553,10 +553,10 @@ export function MeasurementList({ ) { return; } - applyFilters({ ...filters, valueMin: min, valueMax: max }, "replace"); + patchFilters({ valueMin: min, valueMax: max }, "replace"); }, 200); return () => clearTimeout(handle); - }, [valueDraft, filters, applyFilters]); + }, [valueDraft, filters, patchFilters]); const goToPage = (updater: (p: number) => number) => { setPage(updater); diff --git a/src/components/medications/detail/medication-detail-tabs.tsx b/src/components/medications/detail/medication-detail-tabs.tsx index ff57868ce..39a7a8b7b 100644 --- a/src/components/medications/detail/medication-detail-tabs.tsx +++ b/src/components/medications/detail/medication-detail-tabs.tsx @@ -50,7 +50,7 @@ import { } from "react"; import Link from "next/link"; import dynamic from "next/dynamic"; -import { useRouter, useSearchParams } from "next/navigation"; +import { useUrlSearch, writeUrlState } from "@/lib/url-state"; import { ArrowLeft, Pencil } from "lucide-react"; import { Button } from "@/components/ui/button"; @@ -309,8 +309,8 @@ export function MedicationDetailTabs({ const fmt = useFormatters(); const { canManageDomain, inSharedRecord } = useRecordCapabilities(); const canManageMedications = canManageDomain("medications"); - const router = useRouter(); - const searchParams = useSearchParams(); + // Read live (`@/lib/url-state`): the tab is written here. + const searchParams = useUrlSearch(); // v1.15.20 — the medications-list kebab's "Bearbeiten" arrives as // `?edit=1` and opens the wizard straight away (the same editOpen the @@ -407,33 +407,17 @@ export function MedicationDetailTabs({ useEffect(() => { if (shouldOpenEditFromUrl) { // Drop `?edit=1` (keep any `tab` param) so a refresh after closing - // the wizard does not keep reopening it. - const params = new URLSearchParams( - Array.from(searchParams?.entries() ?? []), - ); - params.delete("edit"); - const qs = params.toString(); - router.replace(`/medications/${id}${qs ? `?${qs}` : ""}`, { - scroll: false, - }); + // the wizard does not keep reopening it. Made while mounting, so it + // waits for the router's history integration (`afterPaint`). + writeUrlState((params) => params.delete("edit"), { afterPaint: true }); } - }, [shouldOpenEditFromUrl, id, router, searchParams]); + }, [shouldOpenEditFromUrl]); - const onTabChange = useCallback( - (next: string) => { - const params = new URLSearchParams( - Array.from(searchParams?.entries() ?? []), - ); - params.set("tab", next); - // Shallow URL swap so the tab is deep-linkable + survives reload - // without a full navigation. `scroll: false` keeps the viewport - // pinned to the tab strip on switch. - router.replace(`/medications/${id}?${params.toString()}`, { - scroll: false, - }); - }, - [id, router, searchParams], - ); + const onTabChange = useCallback((next: string) => { + // The tab is deep-linkable and survives a reload; a shallow in-place + // write keeps the viewport pinned to the tab strip on switch. + writeUrlState((params) => params.set("tab", next)); + }, []); const { data: compliance } = useQuery({ queryKey: queryKeys.medicationCompliance(id), diff --git a/src/components/mood/__tests__/mood-list-url-sync.test.tsx b/src/components/mood/__tests__/mood-list-url-sync.test.tsx index 5717de85c..92f11cc33 100644 --- a/src/components/mood/__tests__/mood-list-url-sync.test.tsx +++ b/src/components/mood/__tests__/mood-list-url-sync.test.tsx @@ -10,7 +10,7 @@ * user's click (post-render setState is a no-op under the server renderer; * the router spies record the navigation the click causes). */ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; const baseEntries = [ @@ -101,6 +101,13 @@ vi.mock("@/components/ui/filter-bar", () => ({ import { I18nProvider } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { MoodList } from "../mood-list"; +import { + installUrlStateBrowser, + type UrlStateBrowser, +} from "@/__tests__/helpers/url-state-browser"; + +let browser: UrlStateBrowser = installUrlStateBrowser("/"); +afterEach(() => browser.uninstall()); function render(search: string) { mockSearch = search; @@ -108,11 +115,15 @@ function render(search: string) { barProps = null; dateRange = null; recordedQueryKeys.length = 0; - return renderToStaticMarkup( + const html = renderToStaticMarkup( , ); + // The live address the writes start from (`@/lib/url-state`). + browser.uninstall(); + browser = installUrlStateBrowser(search ? `/mood?${search}` : "/mood"); + return html; } const moodSelect = () => selects[0]!; @@ -127,37 +138,33 @@ describe("MoodList — filter changes write the URL", () => { it("pushes the mood facet", () => { render(""); moodSelect().onValueChange("GUT"); - expect(pushMock).toHaveBeenCalledWith("/mood?mood=GUT", { scroll: false }); + expect(browser.push).toHaveBeenCalledWith("/mood?mood=GUT"); }); it("pushes the source facet on top of the active mood", () => { render("mood=GUT"); sourceSelect().onValueChange("TELEGRAM"); - expect(pushMock).toHaveBeenCalledWith("/mood?mood=GUT&source=TELEGRAM", { - scroll: false, - }); + expect(browser.push).toHaveBeenCalledWith("/mood?mood=GUT&source=TELEGRAM"); }); it("replaces (no history entry) for a committed date bound", () => { render(""); dateRange!.onToChange("2026-05-31"); - expect(replaceMock).toHaveBeenCalledWith("/mood?to=2026-05-31", { - scroll: false, - }); - expect(pushMock).not.toHaveBeenCalled(); + expect(browser.replace).toHaveBeenCalledWith("/mood?to=2026-05-31"); + expect(browser.push).not.toHaveBeenCalled(); }); it("clearing the last facet restores the bare pathname", () => { render("mood=GUT"); moodSelect().onValueChange("ALL"); - expect(pushMock).toHaveBeenCalledWith("/mood", { scroll: false }); + expect(browser.push).toHaveBeenCalledWith("/mood"); }); it("reset clears every facet in one push", () => { render("mood=GUT&source=TELEGRAM&from=2026-05-01"); expect(barProps!.isFiltered).toBe(true); barProps!.onReset(); - expect(pushMock).toHaveBeenCalledWith("/mood", { scroll: false }); + expect(browser.push).toHaveBeenCalledWith("/mood"); }); }); diff --git a/src/components/mood/mood-list.tsx b/src/components/mood/mood-list.tsx index 67102c5f7..c9cc771d0 100644 --- a/src/components/mood/mood-list.tsx +++ b/src/components/mood/mood-list.tsx @@ -197,7 +197,7 @@ export function MoodList({ onAddFirst, onEmptyChange }: MoodListProps = {}) { // through the router, so a filtered view survives navigation, reload and // back/forward. // v1.15.13 — management-list source filter + optional date range. - const { filters, applyFilters } = useUrlFilterSync({ + const { filters, applyFilters, patchFilters } = useUrlFilterSync({ parse: parseMoodListSearchParams, serialise: moodListFiltersToSearch, }); @@ -298,25 +298,25 @@ export function MoodList({ onAddFirst, onEmptyChange }: MoodListProps = {}) { // vault's chips); the committed date bounds replace — a from/to pair set in // one sitting should not cost two Back presses. const setMoodFilter = (value: string) => { - applyFilters({ ...filters, mood: value === "ALL" ? undefined : value }); + patchFilters({ mood: value === "ALL" ? undefined : value }); setPage(1); clearSelection(); }; const setSourceFilter = (value: string) => { - applyFilters({ ...filters, source: value === "ALL" ? undefined : value }); + patchFilters({ source: value === "ALL" ? undefined : value }); setPage(1); clearSelection(); }; const setFromDay = (value: string) => { - applyFilters({ ...filters, fromDay: value || undefined }, "replace"); + patchFilters({ fromDay: value || undefined }, "replace"); setPage(1); clearSelection(); }; const setToDay = (value: string) => { - applyFilters({ ...filters, toDay: value || undefined }, "replace"); + patchFilters({ toDay: value || undefined }, "replace"); setPage(1); clearSelection(); }; diff --git a/src/components/settings/ai/ai-insights-card.tsx b/src/components/settings/ai/ai-insights-card.tsx index 0a1c7897a..e9e793a8c 100644 --- a/src/components/settings/ai/ai-insights-card.tsx +++ b/src/components/settings/ai/ai-insights-card.tsx @@ -14,7 +14,7 @@ */ import { useState } from "react"; -import { usePathname, useRouter, useSearchParams } from "next/navigation"; +import { useSearchParams } from "next/navigation"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { Sparkles } from "lucide-react"; @@ -51,6 +51,7 @@ import { type ProviderType, type UserAIProvider, } from "./shared"; +import { writeUrlState } from "@/lib/url-state"; export function AiInsightsCard({ isAuthenticated, @@ -59,8 +60,6 @@ export function AiInsightsCard({ }) { const { t } = useTranslations(); const queryClient = useQueryClient(); - const router = useRouter(); - const pathname = usePathname(); const searchParams = useSearchParams(); // A read failure must surface, not be swallowed into a null that renders the @@ -146,10 +145,7 @@ export function AiInsightsCard({ function pickProvider(next: ProviderType) { setSelectedProvider(next); - if (typeof window === "undefined") return; - const params = new URLSearchParams(searchParams?.toString() ?? ""); - params.set("provider", next); - router.replace(`${pathname}?${params.toString()}`); + writeUrlState((params) => params.set("provider", next)); } // v1.42 — one card per concern. The page used to be a single "AI provider" diff --git a/src/components/settings/ai/codex-provider-form.tsx b/src/components/settings/ai/codex-provider-form.tsx index e8d0a18c1..2d7446f70 100644 --- a/src/components/settings/ai/codex-provider-form.tsx +++ b/src/components/settings/ai/codex-provider-form.tsx @@ -19,6 +19,7 @@ import { aiInputDependentKeys, invalidateKeys } from "@/lib/query-keys"; import type { InsightsSettings } from "./shared"; import { InlineAction } from "@/components/ui/inline-action"; +import { dropParams, writeUrlState } from "@/lib/url-state"; export function CodexProviderForm({ settings, @@ -52,10 +53,9 @@ export function CodexProviderForm({ useEffect(() => { if (!oauthOutcome) return; - const url = new URL(window.location.href); - url.searchParams.delete("codex_connected"); - url.searchParams.delete("codex_error"); - window.history.replaceState({}, "", url.toString()); + writeUrlState(dropParams("codex_connected", "codex_error"), { + afterPaint: true, + }); if (oauthOutcome.kind === "connected") { void invalidateKeys(queryClient, aiInputDependentKeys); } diff --git a/src/components/settings/integrations/connections-panel.tsx b/src/components/settings/integrations/connections-panel.tsx index f44a30aaf..5e26efb55 100644 --- a/src/components/settings/integrations/connections-panel.tsx +++ b/src/components/settings/integrations/connections-panel.tsx @@ -39,6 +39,7 @@ import { useAuth } from "@/hooks/use-auth"; import type { IntegrationCallbackUrls } from "@/lib/integrations/callback-urls"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; +import { dropParams, writeUrlState } from "@/lib/url-state"; /** * The Withings OAuth callback (`/api/withings/callback`) redirects back @@ -246,10 +247,7 @@ export function ConnectionsPanel({ if (!withingsOauthOutcome) return; // Scrub the one-shot params so a reload / bookmark doesn't replay // the toast. - const url = new URL(window.location.href); - url.searchParams.delete("withings"); - url.searchParams.delete("reason"); - router.replace(`${url.pathname}${url.search}`, { scroll: false }); + writeUrlState(dropParams("withings", "reason"), { afterPaint: true }); if (withingsOauthOutcome.kind === "connected") { toast.success(t("settings.withingsOauthConnected")); queryClient.invalidateQueries({ queryKey: queryKeys.withings() }); @@ -282,10 +280,7 @@ export function ConnectionsPanel({ useEffect(() => { if (!oauthOutcome) return; const { provider } = oauthOutcome; - const url = new URL(window.location.href); - url.searchParams.delete(provider); - url.searchParams.delete("reason"); - router.replace(`${url.pathname}${url.search}`, { scroll: false }); + writeUrlState(dropParams(provider, "reason"), { afterPaint: true }); if (oauthOutcome.kind === "connected") { toast.success(t(`settings.${provider}OauthConnected`)); // Polar/Oura own a per-card status query; WHOOP/Fitbit read off the diff --git a/src/components/timeline/timeline-view.tsx b/src/components/timeline/timeline-view.tsx index 86dc117e9..ac8a9c97b 100644 --- a/src/components/timeline/timeline-view.tsx +++ b/src/components/timeline/timeline-view.tsx @@ -19,7 +19,7 @@ * thin and the person has not closed it; empty lanes are never drawn. */ import { useCallback, useMemo, useState } from "react"; -import { usePathname, useSearchParams } from "next/navigation"; +import { setAllParams, useUrlSearch, writeUrlState } from "@/lib/url-state"; import { CalendarRange, Plus } from "lucide-react"; import { openDay as openDayLayer } from "@/components/day/day-layer-controller"; @@ -151,8 +151,9 @@ export function TimelineView() { const { t } = useTranslations(); const { user } = useAuth(); const caps = useRecordCapabilities(); - const pathname = usePathname(); - const searchParams = useSearchParams(); + // Read live (`@/lib/url-state`): the router's copy trails a write by a + // render, and the zoom, the range and `?day=` are all written here. + const searchParams = useUrlSearch(); const today = todayKeyIn(user?.timezone); const dayParam = parseDayParam(searchParams.get(DAY_QUERY_PARAM), today); // Life events are written in one's own record only (owner-only routes). @@ -285,14 +286,7 @@ export function TimelineView() { if (open) return; setEditing(null); if (searchParams.has("add")) { - const next = new URLSearchParams(searchParams.toString()); - next.delete("add"); - const search = next.toString(); - window.history.replaceState( - null, - "", - search ? `${pathname}?${search}` : pathname, - ); + writeUrlState((params) => params.delete("add")); } } @@ -312,8 +306,13 @@ export function TimelineView() { (range ? range.to : today); function navigate(nextZoom: TimelineZoom, nextRange: TimelineRange | null) { - const search = timelineSearch(window.location.search, nextZoom, nextRange); - window.history.pushState(null, "", `${pathname}${search}`); + writeUrlState( + (params) => { + const next = timelineSearch(`?${params}`, nextZoom, nextRange); + setAllParams(next)(params); + }, + { mode: "push" }, + ); } function setZoom(next: Exclude) { diff --git a/src/hooks/__tests__/use-url-filter-sync.test.tsx b/src/hooks/__tests__/use-url-filter-sync.test.tsx index 36aac48c7..81e5a00bd 100644 --- a/src/hooks/__tests__/use-url-filter-sync.test.tsx +++ b/src/hooks/__tests__/use-url-filter-sync.test.tsx @@ -6,7 +6,7 @@ * request), an empty filter set restores the bare pathname, and * `syncToUrl: false` detaches the state from the URL entirely. */ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; let mockSearch = ""; @@ -19,6 +19,13 @@ vi.mock("next/navigation", () => ({ })); import { useUrlFilterSync } from "../use-url-filter-sync"; +import { + installUrlStateBrowser, + type UrlStateBrowser, +} from "@/__tests__/helpers/url-state-browser"; + +let browser: UrlStateBrowser = installUrlStateBrowser("/"); +afterEach(() => browser.uninstall()); interface ProbeFilters { type?: string; @@ -61,6 +68,11 @@ function mount(search: string, syncToUrl?: boolean): ProbeResult { } renderToStaticMarkup(); if (!captured.current) throw new Error("probe did not render"); + // The live address the writes start from (`@/lib/url-state`). + browser.uninstall(); + browser = installUrlStateBrowser( + search ? `/measurements?${search}` : "/measurements", + ); return captured.current; } @@ -78,32 +90,28 @@ describe("useUrlFilterSync", () => { it("writes a filter change to the URL via push by default", () => { const { applyFilters } = mount(""); applyFilters({ type: "WEIGHT" }); - expect(pushMock).toHaveBeenCalledWith("/measurements?type=WEIGHT", { - scroll: false, - }); - expect(replaceMock).not.toHaveBeenCalled(); + expect(browser.push).toHaveBeenCalledWith("/measurements?type=WEIGHT"); + expect(browser.replace).not.toHaveBeenCalled(); }); it("writes via replace when asked (high-frequency input)", () => { const { applyFilters } = mount(""); applyFilters({ source: "MANUAL" }, "replace"); - expect(replaceMock).toHaveBeenCalledWith("/measurements?source=MANUAL", { - scroll: false, - }); - expect(pushMock).not.toHaveBeenCalled(); + expect(browser.replace).toHaveBeenCalledWith("/measurements?source=MANUAL"); + expect(browser.push).not.toHaveBeenCalled(); }); it("restores the bare pathname when every facet clears", () => { const { applyFilters } = mount("type=WEIGHT"); applyFilters({}); - expect(pushMock).toHaveBeenCalledWith("/measurements", { scroll: false }); + expect(browser.push).toHaveBeenCalledWith("/measurements"); }); it("syncToUrl: false ignores the URL and never navigates", () => { const { filters, applyFilters } = mount("type=WEIGHT", false); expect(filters).toEqual({}); applyFilters({ type: "PULSE" }); - expect(pushMock).not.toHaveBeenCalled(); - expect(replaceMock).not.toHaveBeenCalled(); + expect(browser.push).not.toHaveBeenCalled(); + expect(browser.replace).not.toHaveBeenCalled(); }); }); diff --git a/src/hooks/use-url-filter-sync.ts b/src/hooks/use-url-filter-sync.ts index 0050182ca..bd105bafa 100644 --- a/src/hooks/use-url-filter-sync.ts +++ b/src/hooks/use-url-filter-sync.ts @@ -1,8 +1,9 @@ "use client"; -import { usePathname, useRouter, useSearchParams } from "next/navigation"; import { useCallback, useMemo, useState } from "react"; +import { setAllParams, useUrlSearch, writeUrlState } from "@/lib/url-state"; + /** * URL-owned filter state — the documents-vault pattern * (`src/components/documents/documents-view.tsx`), extracted so the @@ -20,6 +21,13 @@ import { useCallback, useMemo, useState } from "react"; * the vault's kind/year chips; high-frequency typed input passes `replace` so * the history never fills with keystrokes, like the vault's debounced search. * + * Reads and writes go through `@/lib/url-state`: the filters parse from the + * live address bar, and `applyFilters` takes either the next filters or an + * updater that receives the live ones at the moment of the write. A change + * computed from a rendered copy wrote the previous filters back when Next + * adopted an earlier write a render late (clearing the vault's filters left + * `?encounter=` behind). + * * `syncToUrl: false` keeps the identical contract in component memory only — * for a list embedded under a URL that belongs to another surface (the * type-locked insights readings subpage). @@ -34,16 +42,16 @@ export function useUrlFilterSync({ syncToUrl?: boolean; }): { filters: T; - applyFilters: (next: T, mode?: "push" | "replace") => void; + applyFilters: ( + next: T | ((current: T) => T), + mode?: "push" | "replace", + ) => void; + /** Merge `patch` into the live filters (the common facet change). */ + patchFilters: (patch: Partial, mode?: "push" | "replace") => void; } { - const router = useRouter(); - const pathname = usePathname(); - const searchParams = useSearchParams(); + const params = useUrlSearch(); - const urlFilters = useMemo( - () => parse(new URLSearchParams(searchParams.toString())), - [parse, searchParams], - ); + const urlFilters = useMemo(() => parse(params), [parse, params]); // Memory-only fallback for `syncToUrl: false`. The hook always mounts both // sources (hooks are unconditional); only one is read. @@ -52,21 +60,33 @@ export function useUrlFilterSync({ ); const applyFilters = useCallback( - (next: T, mode: "push" | "replace" = "push") => { + (next: T | ((current: T) => T), mode: "push" | "replace" = "push") => { + const resolve = (current: T): T => + typeof next === "function" ? (next as (c: T) => T)(current) : next; if (!syncToUrl) { - setLocalFilters(next); + setLocalFilters(resolve); return; } - const search = serialise(next); - const href = search ? `${pathname}?${search}` : pathname; - if (mode === "push") { - router.push(href, { scroll: false }); - } else { - router.replace(href, { scroll: false }); - } + writeUrlState( + (live) => { + const search = serialise(resolve(parse(live))); + setAllParams(search)(live); + }, + { mode }, + ); }, - [syncToUrl, serialise, pathname, router], + [syncToUrl, serialise, parse], ); - return { filters: syncToUrl ? urlFilters : localFilters, applyFilters }; + const patchFilters = useCallback( + (patch: Partial, mode: "push" | "replace" = "push") => + applyFilters((current) => ({ ...current, ...patch }), mode), + [applyFilters], + ); + + return { + filters: syncToUrl ? urlFilters : localFilters, + applyFilters, + patchFilters, + }; } diff --git a/src/lib/__tests__/url-state.test.ts b/src/lib/__tests__/url-state.test.ts new file mode 100644 index 000000000..c0c0fd38a --- /dev/null +++ b/src/lib/__tests__/url-state.test.ts @@ -0,0 +1,125 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + installUrlStateBrowser, + type UrlStateBrowser, +} from "@/__tests__/helpers/url-state-browser"; + +import { + ADOPTION_GRACE_MS, + currentUrlSearch, + reportRouterSearch, + setAllParams, + writeUrlHref, + writeUrlState, +} from "../url-state"; + +/** + * The URL-state helper's contract: every write starts from the live address + * (never a rendered copy), at most one write waits for the router at a time, + * and what queues behind it is applied on top, together, once the router + * has the first one (or the grace runs out). + */ + +let browser: UrlStateBrowser; +afterEach(() => { + browser.uninstall(); + vi.useRealTimers(); +}); + +describe("writeUrlState", () => { + it("edits the live parameters and writes one entry", () => { + browser = installUrlStateBrowser("/documents?q=scan"); + writeUrlState((params) => params.set("encounter", "e1"), { + mode: "push", + }); + expect(browser.push).toHaveBeenCalledWith("/documents?q=scan&encounter=e1"); + expect(browser.replace).not.toHaveBeenCalled(); + }); + + it("does not write when nothing changes", () => { + browser = installUrlStateBrowser("/documents?q=scan"); + writeUrlState((params) => params.set("q", "scan")); + expect(browser.replace).not.toHaveBeenCalled(); + }); + + it("holds a second write until the router has the first, then applies both", () => { + browser = installUrlStateBrowser("/documents?q=scan", { + autoAdopt: false, + }); + writeUrlState((params) => params.set("encounter", "e1"), { + mode: "push", + }); + writeUrlState((params) => params.set("year", "2025")); + // Not yet written, but every reader already sees it. + expect(browser.current()).toBe("/documents?q=scan&encounter=e1"); + expect(currentUrlSearch()).toBe("?q=scan&encounter=e1&year=2025"); + reportRouterSearch("?q=scan&encounter=e1"); + expect(browser.current()).toBe("/documents?q=scan&encounter=e1&year=2025"); + }); + + it("never puts the cleared filters back (the vault's ?encounter= case)", () => { + browser = installUrlStateBrowser("/documents?q=scan&encounter=e1", { + autoAdopt: false, + }); + // "Clear filters" empties the query … + writeUrlState(setAllParams(""), { mode: "push" }); + // … and the debounced search, still holding the rendered filters, + // writes `q` again. Derived from the live query, it keeps the clear. + writeUrlState((params) => params.delete("q"), { mode: "replace" }); + reportRouterSearch(""); + expect(browser.current()).toBe("/documents"); + expect(browser.current()).not.toMatch(/encounter=/); + }); + + it("lets the queue go after the grace when no reader reports", () => { + vi.useFakeTimers(); + browser = installUrlStateBrowser("/timeline", { autoAdopt: false }); + writeUrlState((params) => params.set("zoom", "year"), { mode: "push" }); + writeUrlState((params) => params.set("day", "2026-01-03")); + expect(browser.current()).toBe("/timeline?zoom=year"); + vi.advanceTimersByTime(ADOPTION_GRACE_MS); + expect(browser.current()).toBe("/timeline?zoom=year&day=2026-01-03"); + }); + + it("coalesces a queued push with a queued replace into one new entry", () => { + browser = installUrlStateBrowser("/mood", { autoAdopt: false }); + writeUrlState((params) => params.set("mood", "GUT")); + writeUrlState((params) => params.set("source", "MANUAL"), { mode: "push" }); + writeUrlState((params) => params.set("from", "2026-05-01")); + reportRouterSearch("?mood=GUT"); + expect(browser.push).toHaveBeenCalledTimes(1); + expect(browser.current()).toBe( + "/mood?mood=GUT&source=MANUAL&from=2026-05-01", + ); + }); + + it("does not wait on a write that keeps the router's own entry state", () => { + browser = installUrlStateBrowser("/coach?settings=data", { + autoAdopt: false, + }); + writeUrlState((params) => params.delete("settings"), { state: "keep" }); + writeUrlState((params) => params.set("c", "abc"), { state: "keep" }); + expect(browser.current()).toBe("/coach?c=abc"); + }); + + it("writes a whole href through the same queue", () => { + browser = installUrlStateBrowser("/coach?c=old"); + writeUrlHref("/coach?c=new"); + expect(browser.replace).toHaveBeenCalledWith("/coach?c=new"); + }); + + it("waits two frames for a write made while mounting", () => { + browser = installUrlStateBrowser("/measurements?add=WEIGHT&type=PULSE"); + const raf = vi.fn((callback: (time: number) => void) => { + callback(0); + return 0; + }); + ( + window as unknown as { requestAnimationFrame: typeof raf } + ).requestAnimationFrame = raf; + writeUrlState((params) => params.delete("add"), { afterPaint: true }); + expect(raf).toHaveBeenCalledTimes(2); + expect(browser.current()).toBe("/measurements?type=PULSE"); + }); +}); diff --git a/src/lib/url-state.ts b/src/lib/url-state.ts new file mode 100644 index 000000000..3a7de1cf6 --- /dev/null +++ b/src/lib/url-state.ts @@ -0,0 +1,271 @@ +"use client"; + +/** + * The one way a page writes its own state into the query string. + * + * Next 16.4 adopts a native `history.pushState` / `replaceState` a render + * after the write: its router commits its copy of the URL then, and + * `useSearchParams` follows only at that commit. Two things went wrong + * around that gap, on every surface that kept state in the URL: + * + * - a second write that landed inside the gap was undone when the router + * adopted the first one and put the first URL back (`[` then `]` on the + * day ended a day short; clearing the document filters left + * `?encounter=` behind); + * - a write computed from `useSearchParams` started from the URL before + * the previous write, and so wrote the old parameters back. + * + * So every write here derives from the live address bar at the moment it is + * made, never from a rendered copy, and at most one write is in flight at a + * time: while the router has not adopted the last one, further updates are + * queued and applied together, on top of it, once it has (or after a short + * grace when no reader reports it). Readers go through `useUrlSearch`, which + * reads the same live value, including updates still queued, and reports + * each adoption back. + * + * Writes the page makes as it mounts (stripping a one-shot `?add=`) pass + * `afterPaint`: the router installs its native-history integration a frame + * or two after the first effects run, and a write before that loses the + * router's own entry state. + */ +import { useSearchParams } from "next/navigation"; +import { useEffect, useMemo, useSyncExternalStore } from "react"; + +export type UrlStateUpdate = (params: URLSearchParams) => void; + +export interface UrlStateWriteOptions { + /** A new history entry (`push`) or the current one rewritten (`replace`). */ + mode?: "push" | "replace"; + /** + * The history entry's state: an object to store, or `"keep"` for the + * current entry's own state. Defaults to `null`, which the router fills + * with its own. + */ + state?: unknown; + /** Wait two painted frames before writing (a write made while mounting). */ + afterPaint?: boolean; + /** Write to this path instead of the current one (same page, new path). */ + pathname?: string; +} + +/** How long a write waits for the router to report it before the next goes. */ +export const ADOPTION_GRACE_MS = 250; + +/** The event every write dispatches, so `useUrlSearch` re-reads at once. */ +export const URL_STATE_EVENT = "healthlog:url-state"; + +interface Queued { + updates: UrlStateUpdate[]; + mode: "push" | "replace"; + state: unknown; + pathname: string | null; +} + +/** The query string written last and not yet adopted by the router. */ +let inFlight: string | null = null; +let graceTimer: ReturnType | null = null; +let queued: Queued | null = null; + +function normalise(search: string): string { + if (search === "" || search === "?") return ""; + return search.startsWith("?") ? search : `?${search}`; +} + +function applyUpdates( + search: string, + updates: readonly UrlStateUpdate[], +): string { + const params = new URLSearchParams(search); + for (const update of updates) update(params); + return normalise(params.toString()); +} + +function notify(): void { + window.dispatchEvent(new Event(URL_STATE_EVENT)); +} + +function armGrace(): void { + if (graceTimer !== null) clearTimeout(graceTimer); + graceTimer = setTimeout(() => { + graceTimer = null; + settle(); + }, ADOPTION_GRACE_MS); +} + +/** The router has the last write (or the grace ran out): send what waits. */ +function settle(): void { + inFlight = null; + if (graceTimer !== null) { + clearTimeout(graceTimer); + graceTimer = null; + } + const next = queued; + queued = null; + if (next) commit(next.updates, next.mode, next.state, next.pathname); +} + +function commit( + updates: readonly UrlStateUpdate[], + mode: "push" | "replace", + state: unknown, + pathname: string | null, +): void { + const { location, history } = window; + const search = applyUpdates(location.search, updates); + const path = pathname ?? location.pathname; + if (search === normalise(location.search) && path === location.pathname) { + notify(); + return; + } + const entryState = state === "keep" ? history.state : (state ?? null); + const href = `${path}${search}${location.hash}`; + if (mode === "push") history.pushState(entryState, "", href); + else history.replaceState(entryState, "", href); + // A write that keeps the router's own entry state is one the router does + // not adopt (it reads it as its own), so nothing waits for it. + if (state !== "keep") { + inFlight = search; + armGrace(); + } + notify(); +} + +/** + * Change the query string. `update` receives the live parameters (with any + * update still queued already applied) and edits them in place. + */ +export function writeUrlState( + update: UrlStateUpdate, + options: UrlStateWriteOptions = {}, +): void { + if (typeof window === "undefined") return; + const mode = options.mode ?? "replace"; + if (options.afterPaint) { + const { afterPaint: _deferred, ...rest } = options; + void _deferred; + window.requestAnimationFrame(() => + window.requestAnimationFrame(() => writeUrlState(update, rest)), + ); + return; + } + const pathname = options.pathname ?? null; + if (inFlight !== null) { + queued = { + updates: [...(queued?.updates ?? []), update], + // One coalesced write: a new entry if any of them asked for one. + mode: queued?.mode === "push" || mode === "push" ? "push" : "replace", + state: options.state ?? queued?.state ?? null, + pathname: pathname ?? queued?.pathname ?? null, + }; + notify(); + return; + } + commit([update], mode, options.state ?? null, pathname); +} + +/** + * Write a whole same-origin href (path and query) computed by a page's own + * URL builder. Prefer `writeUrlState` with an update for state derived from + * the current query: this form replaces the query outright. + */ +export function writeUrlHref( + href: string, + options: Omit = {}, +): void { + if (typeof window === "undefined") return; + const target = new URL(href, window.location.href); + writeUrlState(setAllParams(target.search), { + ...options, + pathname: target.pathname, + }); +} + +/** + * A `history`-shaped view for code written against the History API (the + * vault's close path takes one, so its tests can pass a fake): `state` and + * `back` are the browser's, `replaceState` goes through `writeUrlState`. + */ +export const urlStateHistory = { + get state(): unknown { + return window.history.state; + }, + back(): void { + window.history.back(); + }, + replaceState(data: unknown, _unused: string, url?: string | URL | null) { + if (url === undefined || url === null) return; + writeUrlHref(String(url), { mode: "replace", state: data }); + }, +}; + +/** Drop the named parameters (a one-shot `?add=`, an OAuth outcome). */ +export function dropParams(...keys: string[]): UrlStateUpdate { + return (params) => { + for (const key of keys) params.delete(key); + }; +} + +/** Replace every parameter with `search` (a list's whole filter state). */ +export function setAllParams(search: string): UrlStateUpdate { + return (params) => { + for (const key of [...params.keys()]) params.delete(key); + new URLSearchParams(search).forEach((value, key) => { + params.append(key, value); + }); + }; +} + +/** + * The live query string, `?`-prefixed or empty: the address bar with any + * update still waiting in the queue applied on top. + */ +export function currentUrlSearch(): string { + if (typeof window === "undefined") return ""; + const live = normalise(window.location.search); + return queued ? applyUpdates(live, queued.updates) : live; +} + +/** + * The router's `useSearchParams` now shows `search`. Called by every + * `useUrlSearch` reader; once it matches the write in flight, the queue + * moves. + */ +export function reportRouterSearch(search: string): void { + if (inFlight !== null && normalise(search) === inFlight) settle(); +} + +function subscribe(onChange: () => void): () => void { + window.addEventListener("popstate", onChange); + window.addEventListener(URL_STATE_EVENT, onChange); + return () => { + window.removeEventListener("popstate", onChange); + window.removeEventListener(URL_STATE_EVENT, onChange); + }; +} + +/** + * The page's query parameters, read live: the address bar (and any queued + * update) rather than the router's copy, which trails a write by a render. + * Before hydration the router's copy stands in. + */ +export function useUrlSearch(): URLSearchParams { + const routerParams = useSearchParams(); + const routerSearch = normalise(routerParams?.toString() ?? ""); + useEffect(() => { + reportRouterSearch(routerSearch); + }, [routerSearch]); + const search = useSyncExternalStore( + subscribe, + currentUrlSearch, + () => routerSearch, + ); + return useMemo(() => new URLSearchParams(search), [search]); +} + +/** Test seam: forget any write in flight or queued. */ +export function resetUrlStateForTests(): void { + inFlight = null; + queued = null; + if (graceTimer !== null) clearTimeout(graceTimer); + graceTimer = null; +}