fix(nav): the address follows the view through overlays, reloads and Back (review round 1)

- A URL mirror names the view again on every entry Back or Forward lands on, so a change made under a dialog, a drawer or a sub-view is not undone by the next reload. - A view pushed from the entry a reload under a dialog started from no longer inherits its overlay mark; a reload under two overlays closes onto the page in one step. - The linked player reopens over a live search, gives itself up when its list moves first, and plays alone over an emptied playlist; a capped Files list keeps the link it cannot resolve. - The Feed press over a channel ends the search; leaving a search ends its asked mark; Files and Playlists searches end with their page; the feed always names its view and debounces its search. - Search-box keys are declared per module and kept out of the client log below TRACE. - Writers outside lib/ go through codecs; the URL contract scans computed keys, overlay writers and two-line reads, and holds every page to a bookmark round trip (Config, Activity, Playlists and the Files sort and viewer added).
2026-10-27 13:52:42 +01:00
parent 7b4e007680
commit 0d5c6273f1
50 changed files with 928 additions and 157 deletions
@@ -3,26 +3,28 @@
 // The URL is the view: each module declares the address-bar keys its view writes (`urlKeys`), and the
 // shell clears them on a page switch and opens a page without another page's keys (NavigationProvider,
 // derived from these declarations). That only holds while the declarations are true, so this pins
-// them: each key belongs to one module and is no shell key, every codec writes only its module's keys,
-// and no key literal anywhere in the app reads or writes the address bar undeclared.
+// them: each key belongs to one module and is no shell key, the keys a module declares are exactly
+// what its encoders write, a writer outside lib/ hands the address an encoder's result (or literal
+// keys, checked here) — never a key of its own making — and every key literal read from the live
+// query is declared.
 import { describe, it, expect } from "vitest";
 import { readdirSync, readFileSync, statSync } from "node:fs";
 import { join, relative } from "node:path";
-import { moduleDef, REGISTERED_PAGES } from "./features/navigation/moduleRegistry";
+import { moduleDef, REGISTERED_PAGES, typedUrlKeys } from "./features/navigation/moduleRegistry";
 import { DEFAULT_TAG_MODES, EMPTY_PLEX_FILTERS, type FeedFilters } from "./lib/api";
 import { filterParamUpdates, SHELL_KEYS, type Page } from "./lib/urlState";
 import { channelsParamUpdates } from "./lib/channelsUrlState";
 import { plexParamUpdates, plexSearchParamUpdates } from "./lib/plexUrlState";
 import { activityParamUpdates, DEFAULT_ACTIVITY_STATE } from "./lib/activityUrlState";
 import { DEFAULT_LOGS_STATE, logsParamUpdates } from "./lib/logsUrlState";
-import { FILES_KEYS } from "./lib/filesUrlState";
+import { filesListParams, filesOpenParams, filesSortParams } from "./lib/filesUrlState";
 import { ytSearchParams } from "./lib/ytSearchUrlState";
 import { budgetParams } from "./lib/homeUrlState";
 import { PLAYER_KEYS, playerParamUpdates } from "./lib/playerUrlState";
-import { playlistViewParamUpdates } from "./lib/playlistsUrlState";
-import { SETTINGS_TAB_KEY } from "./lib/settingsTab";
-import { CONFIG_TAB_KEY } from "./lib/configTab";
-import { USERS_TAB_URL_KEY } from "./lib/adminUsersTab";
+import { playlistsParams, playlistViewParamUpdates } from "./lib/playlistsUrlState";
+import { settingsTabParams } from "./lib/settingsTab";
+import { configTabParams } from "./lib/configTab";
+import { usersTabParams } from "./lib/adminUsersTab";
 import { PLEX_SUB_URL } from "./modules/plex/plexSubUrl";
 import { MODE_URL } from "./shell/welcomeMode";
 import { CHANNEL_PAGE_KEYS, channelPageParamUpdates } from "./lib/channelPageUrlState";
@@ -56,13 +58,17 @@ const WRITES: Partial<Record<Page, () => string[]>> = {
    ...Object.keys(plexSearchParamUpdates({ q: "x", field: "all" })),
    PLEX_SUB_URL.key,
  ],
-  files: () => [...FILES_KEYS],
-  settings: () => [SETTINGS_TAB_KEY],
-  config: () => [CONFIG_TAB_KEY],
-  users: () => [USERS_TAB_URL_KEY],
+  files: () => [
+    ...Object.keys(filesListParams(null)),
+    ...Object.keys(filesSortParams(null)),
+    ...Object.keys(filesOpenParams({ open: "view", id: 1 })),
+    ...Object.keys(filesOpenParams({ open: "play", id: 1 })),
+  ],
+  settings: () => Object.keys(settingsTabParams(null)),
+  config: () => Object.keys(configTabParams(null)),
+  users: () => Object.keys(usersTabParams(null)),
  playlists: () => [
-    "pl",
-    "plq",
+    ...Object.keys(playlistsParams(null, "")),
    ...Object.keys(playlistViewParamUpdates({ sort: null, filters: {}, groupBy: false })),
  ],
  activity: () => [
@@ -79,25 +85,57 @@ function sources(dir: string): string[] {
  });
 }

-// Key literals that reach the address bar's QUERY: the keys of an object literal handed to
-// `urlWithParams` / `useUrlMirror`, and a `.get("…")` on the live query.
+// The object literals handed to an address writer: `urlWithParams` / `useUrlMirror`, an overlay's
+// own keys (ui/Modal's `url`, `useBackToClose`'s).
+const WRITER_OBJECTS = [
+  /(?:urlWithParams|useUrlMirror)\(\{([^}]*)\}/g,
+  /\burl=\{\{([^}]*)\}/g,
+  /useBackToClose\([^,()]+,\s*\{([^}]*)\}/g,
+];
+
+// Key literals that reach the address bar's QUERY: the literal keys of a writer's object, and a
+// `.get("…")` / `.has("…")` in a file that reads the live query (any params object there: the
+// two-line form names its params before it reads them). The fragment (`hash`) is not the query.
 function queryKeyLiterals(): { key: string; where: string }[] {
  const out: { key: string; where: string }[] = [];
  for (const file of sources(SRC)) {
    const text = readFileSync(file, "utf8");
    const where = relative(SRC, file).split("\\").join("/");
-    for (const m of text.matchAll(/(?:urlWithParams|useUrlMirror)\(\{([^}]*)\}/g)) {
-      for (const k of `{${m[1]!}`.matchAll(/[{,]\s*(\w+)\s*:/g)) out.push({ key: k[1]!, where });
+    for (const re of WRITER_OBJECTS) {
+      for (const m of text.matchAll(re)) {
+        for (const k of `{${m[1]!}`.matchAll(/[{,]\s*(\w+)\s*:/g)) out.push({ key: k[1]!, where });
+      }
    }
-    for (const m of text.matchAll(
-      /new URLSearchParams\(window\.location\.search\)\.get\("(\w+)"\)/g,
-    )) {
-      out.push({ key: m[1]!, where });
+    if (!text.includes("location.search")) continue;
+    for (const m of text.matchAll(/(\b\w+|\))\.(?:get|has)\("(\w+)"\)/g)) {
+      if (m[1] !== "hash") out.push({ key: m[2]!, where });
    }
  }
  return out;
 }

+// A key a writer computes itself (`[KEY]: …`) is one no scan can read: outside lib/ (the codecs, and
+// the history plumbing they feed) a writer hands the address an encoder's result instead, and the
+// encoder is what WRITES calls.
+function computedWriterKeys(inLib = false): string[] {
+  const out: string[] = [];
+  for (const file of sources(SRC)) {
+    const where = relative(SRC, file).split("\\").join("/");
+    if (where.startsWith("lib/") !== inLib) continue;
+    const text = readFileSync(file, "utf8");
+    for (const re of WRITER_OBJECTS) {
+      for (const m of text.matchAll(re)) {
+        if (/[{,]\s*\[/.test(`{${m[1]!}`)) out.push(`${where}: ${m[0]!.slice(0, 60)}`);
+      }
+    }
+  }
+  return out;
+}
+
+// A retired page's link keys: read once to open the page that replaced it, and cleared (lib/urlState
+// RETIRED) — no module writes them any more.
+const RETIRED_LINK_KEYS = ["lu", "lq", "lwin"];
+
 describe("module URL keys", () => {
  const owner = new Map<string, Page>();
  for (const page of REGISTERED_PAGES) {
@@ -154,14 +192,62 @@ describe("module URL keys", () => {
  it("cover every key literal the app reads or writes in the address bar", () => {
    const shell: readonly string[] = SHELL_KEYS;
    const undeclared = queryKeyLiterals().filter(
-      ({ key }) => !owner.has(key) && !shell.includes(key),
+      ({ key }) => !owner.has(key) && !shell.includes(key) && !RETIRED_LINK_KEYS.includes(key),
    );
    expect(undeclared).toEqual([]);
  });

  it("the literal scan sees the call sites it is meant to (a regex that matched nothing would pass)", () => {
    const seen = new Set(queryKeyLiterals().map(({ key }) => key));
-    for (const key of ["pl", "page", "rjob", "link"]) expect(seen).toContain(key);
+    // A writer's literal, a one-line read, a two-line read, a retired link's.
+    for (const key of ["page", "rjob", "link", "plq", "folder", "lq"]) expect(seen).toContain(key);
+  });
+
+  it("are written outside lib/ only through the codecs that WRITES calls", () => {
+    expect(computedWriterKeys()).toEqual([]);
+  });
+
+  it("the computed-key scan sees what it is meant to (lib's own writers compute theirs)", () => {
+    expect(computedWriterKeys(true).some((w) => w.startsWith("lib/history.ts"))).toBe(true);
+  });
+
+  // What a search box puts in the address is what the user typed: the client log keeps it out below
+  // its most detailed level (lib/logHooks), from these declarations.
+  it("declare their typed-text keys among their own", () => {
+    for (const page of REGISTERED_PAGES) {
+      const def = moduleDef(page);
+      expect(
+        (def.typedUrlKeys ?? []).filter((k) => !def.urlKeys.includes(k)),
+        page,
+      ).toEqual([]);
+    }
+    expect([...typedUrlKeys()].sort()).toEqual(["aq", "cq", "fq", "plq", "pq", "q", "yt"]);
+    // …which the client log is handed as it installs.
+    expect(readFileSync(join(SRC, "main.tsx"), "utf8")).toContain(
+      "installClientLog(appCopyOf, typedUrlKeys());",
+    );
+  });
+});
+
+// The bookmark net (e2e/bookmark.spec.ts) opens every page that names its view, through the rail or
+// by its address — Plex excepted, by name there: its view is the instance's own library.
+describe("the bookmark net", () => {
+  const spec = readFileSync(join(SRC, "..", "e2e", "bookmark.spec.ts"), "utf8");
+  const opened = (page: Page) =>
+    spec.includes(`page=${page}`) ||
+    spec.includes(`gotoPage(page, "${page}")`) ||
+    spec.includes(`gotoAdminPage(page, "${page}")`);
+
+  it.each(REGISTERED_PAGES.filter((p) => moduleDef(p).urlKeys.length > 0 && p !== "plex"))(
+    "takes a round trip through %s",
+    (page) => {
+      expect(opened(page)).toBe(true);
+    },
+  );
+
+  it("names the page it leaves out", () => {
+    expect(opened("plex")).toBe(false);
+    expect(spec).toContain("Not Plex:");
  });
 });