mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-07 04:12:04 +00:00
fix(settings): forward moved anchors on hash changes too, prove targets exist
A same-page link to a moved anchor changes only the hash, so the arrival check never forwarded it; the shell now also listens for hashchange. The legacy-routes guard walks the component tree of every forward target's page and fails when the anchor it forwards to is not rendered there.
This commit is contained in:
1 parent
b25ee09f7d
commit
709d28523c
3 files changed
+152
-3
No files matched your search
@@ -13,6 +13,9 @@
|
||||
* A route removed without a redirect, a redirect pointing at a page that no
|
||||
* longer exists, or a forward whose target 404s fails here by name.
|
||||
*/
|
||||
import { existsSync, readFileSync, statSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import nextConfig from "../../next.config";
|
||||
@@ -160,6 +163,74 @@ describe("historical Settings addresses", () => {
|
||||
});
|
||||
});
|
||||
|
||||
const SRC = join(process.cwd(), "src");
|
||||
|
||||
/** The component file a Settings page renders from. */
|
||||
function entryFilesFor(page: string): string[] {
|
||||
const parts = page.split("/").filter(Boolean);
|
||||
if (parts.length === 2) {
|
||||
const loader = readFileSync(
|
||||
join(SRC, "components/settings/settings-section-loader.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
const match = new RegExp(
|
||||
`(?:^|\\s)"?${parts[1]}"?:\\s*\\(\\)\\s*=>\\s*import\\("@/([^"]+)"\\)`,
|
||||
"m",
|
||||
).exec(loader);
|
||||
return match ? [resolveModule(match[1]!)].filter(isString) : [];
|
||||
}
|
||||
if (parts[1] === "modules") {
|
||||
return [join(SRC, "components/settings/module-settings-pages.tsx")];
|
||||
}
|
||||
if (parts[1] === "layout") {
|
||||
return [join(SRC, "components/settings/layout-section.tsx")];
|
||||
}
|
||||
return [];
|
||||
}
|
||||
|
||||
function isString(value: string | null): value is string {
|
||||
return value !== null;
|
||||
}
|
||||
|
||||
function resolveModule(spec: string, from?: string): string | null {
|
||||
const base = spec.startsWith(".")
|
||||
? join(dirname(from!), spec)
|
||||
: join(SRC, spec.replace(/^@\//, ""));
|
||||
for (const candidate of [
|
||||
base,
|
||||
`${base}.tsx`,
|
||||
`${base}.ts`,
|
||||
join(base, "index.tsx"),
|
||||
join(base, "index.ts"),
|
||||
]) {
|
||||
if (existsSync(candidate) && statSync(candidate).isFile()) return candidate;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Does any component reachable from `entries` render `anchor`, as a card
|
||||
* header's `anchor` prop or an element id? Walks the component imports only
|
||||
* (`@/components/...` and relative), which is where a card's anchor lives.
|
||||
*/
|
||||
function anchorIsRendered(entries: string[], anchor: string): boolean {
|
||||
const pattern = new RegExp(`(?:anchor|id)=["{]+"?${anchor}"`);
|
||||
const seen = new Set<string>();
|
||||
const queue = [...entries];
|
||||
while (queue.length > 0 && seen.size < 600) {
|
||||
const file = queue.shift()!;
|
||||
if (seen.has(file)) continue;
|
||||
seen.add(file);
|
||||
const text = readFileSync(file, "utf8");
|
||||
if (pattern.test(text)) return true;
|
||||
for (const m of text.matchAll(/from\s+"((?:@\/components\/|\.)[^"]+)"/g)) {
|
||||
const next = resolveModule(m[1]!, file);
|
||||
if (next && !seen.has(next)) queue.push(next);
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
describe("anchors forwarded after a card moved", () => {
|
||||
const forwards = Object.entries(SETTINGS_ANCHOR_FORWARDS).flatMap(
|
||||
([page, anchors]) =>
|
||||
@@ -183,6 +254,26 @@ describe("anchors forwarded after a card moved", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it.each(forwards)(
|
||||
"$target carries the anchor it is forwarded to",
|
||||
({ target }) => {
|
||||
const [page, anchor] = target.split("#");
|
||||
if (!anchor) return;
|
||||
const entries = entryFilesFor(page!);
|
||||
expect(entries.length, page).toBeGreaterThan(0);
|
||||
expect(
|
||||
anchorIsRendered(entries, anchor),
|
||||
`${anchor} not found on ${page}`,
|
||||
).toBe(true);
|
||||
},
|
||||
);
|
||||
|
||||
it("the anchor search finds a known anchor and misses a made-up one", () => {
|
||||
const entries = entryFilesFor("/settings/security");
|
||||
expect(anchorIsRendered(entries, "sessions")).toBe(true);
|
||||
expect(anchorIsRendered(entries, "no-such-anchor-anywhere")).toBe(false);
|
||||
});
|
||||
|
||||
it("leaves an anchor that did not move alone", () => {
|
||||
expect(forwardedAnchorTarget("/settings/privacy", "#retention")).toBeNull();
|
||||
expect(forwardedAnchorTarget("/settings/security", "#sessions")).toBeNull();
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
vi.mock("next/navigation", () => ({
|
||||
usePathname: () => "/settings/privacy",
|
||||
useRouter: () => ({ replace: vi.fn() }),
|
||||
}));
|
||||
|
||||
import { watchAnchorForwards } from "../settings-anchor-forward";
|
||||
|
||||
function fakeWindow(hash: string) {
|
||||
const target = new EventTarget();
|
||||
return Object.assign(target, { location: { hash } });
|
||||
}
|
||||
|
||||
describe("watchAnchorForwards", () => {
|
||||
it("forwards a moved anchor on arrival", () => {
|
||||
const replace = vi.fn();
|
||||
watchAnchorForwards("/settings/privacy", replace, fakeWindow("#sessions"));
|
||||
expect(replace).toHaveBeenCalledWith("/settings/security#sessions");
|
||||
});
|
||||
|
||||
it("forwards when a same-page link changes only the hash", () => {
|
||||
const replace = vi.fn();
|
||||
const win = fakeWindow("#retention");
|
||||
const stop = watchAnchorForwards("/settings/privacy", replace, win);
|
||||
expect(replace).not.toHaveBeenCalled();
|
||||
|
||||
win.location.hash = "#trusted-devices";
|
||||
win.dispatchEvent(new Event("hashchange"));
|
||||
expect(replace).toHaveBeenCalledWith("/settings/security#trusted-devices");
|
||||
|
||||
stop();
|
||||
win.location.hash = "#sessions";
|
||||
win.dispatchEvent(new Event("hashchange"));
|
||||
expect(replace).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -7,7 +7,8 @@
|
||||
* moved while its page stayed cannot be: the `#anchor` never reaches the
|
||||
* server. Bookmarks, cross-links in older notifications and the docs point at
|
||||
* `/settings/privacy#sessions` and the like, so the shell forwards a known
|
||||
* anchor on a known page to the card's new home, once, on arrival.
|
||||
* anchor on a known page to the card's new home, on arrival and when a
|
||||
* same-page link changes only the hash.
|
||||
*
|
||||
* `SETTINGS_ANCHOR_FORWARDS` is also read by the legacy-routes guard, which
|
||||
* proves every target lands on a page that exists.
|
||||
@@ -56,14 +57,34 @@ export function forwardedAnchorTarget(
|
||||
return SETTINGS_ANCHOR_FORWARDS[pathname]?.[anchor] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Forward now, and again whenever only the hash changes: a same-page link to
|
||||
* a moved anchor is no navigation, so an arrival check alone never sees it.
|
||||
* Returns the unsubscribe.
|
||||
*/
|
||||
export function watchAnchorForwards(
|
||||
pathname: string,
|
||||
replace: (target: string) => void,
|
||||
win: Pick<Window, "addEventListener" | "removeEventListener"> & {
|
||||
location: { hash: string };
|
||||
} = window,
|
||||
): () => void {
|
||||
const forward = () => {
|
||||
const target = forwardedAnchorTarget(pathname, win.location.hash);
|
||||
if (target) replace(target);
|
||||
};
|
||||
forward();
|
||||
win.addEventListener("hashchange", forward);
|
||||
return () => win.removeEventListener("hashchange", forward);
|
||||
}
|
||||
|
||||
export function SettingsAnchorForward() {
|
||||
const pathname = usePathname();
|
||||
const router = useRouter();
|
||||
|
||||
useEffect(() => {
|
||||
if (!pathname) return;
|
||||
const target = forwardedAnchorTarget(pathname, window.location.hash);
|
||||
if (target) router.replace(target);
|
||||
return watchAnchorForwards(pathname, (target) => router.replace(target));
|
||||
}, [pathname, router]);
|
||||
|
||||
return null;
|
||||
|
||||
Reference in new issue
Block a user