From 7b408b1c845a8cb17295ce3f7ad046ecf628e794 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 26 May 2026 15:02:02 +1000 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20useRouter().pathname=20=E2=86=92=20?= =?UTF-8?q?usePathname()=20(#928)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 (1M context) --- src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts | 4 +++- src/hooks/useUpdateURLCatalogParam.ts | 8 ++++++++ src/views/ExportMethodView/exportMethodView.tsx | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts b/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts index 393b57c5..afc241eb 100644 --- a/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts +++ b/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts @@ -1,3 +1,4 @@ +import { usePathname } from "next/navigation"; import { NextRouter, useRouter } from "next/router"; import { useEffect } from "react"; import { ROUTER_METHOD } from "../../../../providers/exploreState/actions/stateToUrl/types"; @@ -10,7 +11,8 @@ export const useStateSync = ({ dispatch, state, }: UseStateSyncManagerProps): void => { - const { basePath, isReady, pathname, query } = useRouter(); + const { basePath, isReady, query } = useRouter(); + const pathname = usePathname() ?? ""; const { onClearPopRef, popRef } = useWasPop(); // Extract the query from the state. diff --git a/src/hooks/useUpdateURLCatalogParam.ts b/src/hooks/useUpdateURLCatalogParam.ts index e8e8ef2b..1477aa60 100644 --- a/src/hooks/useUpdateURLCatalogParam.ts +++ b/src/hooks/useUpdateURLCatalogParam.ts @@ -7,6 +7,14 @@ import { useExploreState } from "./useExploreState"; */ export const useUpdateURLCatalogParams = (): void => { const { exploreState } = useExploreState(); + // `pathname` is intentionally still read from useRouter() here so it stays as + // the route pattern (e.g. `/[entityListType]/[entityId]`). The pattern is + // required by the Router.replace({ pathname, query }) call below — Next.js + // interpolates dynamic-param keys (entityListType, entityId, …) out of the + // query into the path. usePathname() would return the already-resolved URL, + // which on dynamic routes would leave those keys in the query string. The + // migration to usePathname()/router.replace() is tracked in #930 and #931 + // and will be done together with the Router.replace refactor. const { basePath, pathname, query } = useRouter(); const { catalogState } = exploreState; diff --git a/src/views/ExportMethodView/exportMethodView.tsx b/src/views/ExportMethodView/exportMethodView.tsx index 428db4c8..92272e98 100644 --- a/src/views/ExportMethodView/exportMethodView.tsx +++ b/src/views/ExportMethodView/exportMethodView.tsx @@ -1,4 +1,4 @@ -import { useRouter } from "next/router"; +import { usePathname } from "next/navigation"; import { JSX } from "react"; import { ComponentCreator } from "../../components/ComponentCreator/ComponentCreator"; import { BackPageView } from "../../components/Layout/components/BackPage/backPageView"; @@ -8,7 +8,7 @@ import { useUpdateURLSearchParams } from "../../hooks/useUpdateURLSearchParams"; export const ExportMethodView = (): JSX.Element => { useUpdateURLSearchParams(); - const { pathname } = useRouter(); + const pathname = usePathname() ?? ""; const { exportMethods, tabs } = useExportConfig(); const { sideColumn } = tabs[0]; const { mainColumn, top } = From 5dc00b52efb977e2fe3ac8f038dffe8895951420 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 26 May 2026 15:44:12 +1000 Subject: [PATCH 2/4] test: cover wasPop and friends in UseStateSync/utils (#928) Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/stateSyncManager_utils.test.ts | 133 +++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 tests/stateSyncManager_utils.test.ts diff --git a/tests/stateSyncManager_utils.test.ts b/tests/stateSyncManager_utils.test.ts new file mode 100644 index 00000000..6fe567a0 --- /dev/null +++ b/tests/stateSyncManager_utils.test.ts @@ -0,0 +1,133 @@ +import { + hasParams, + isSynced, + stringifyQuery, + wasPop, +} from "../src/hooks/stateSyncManager/hooks/UseStateSync/utils"; +import { NextHistoryState } from "../src/services/beforePopState/types"; + +/** + * Builds a minimal NextHistoryState for tests. + * @param url - The resolved URL the browser is navigating to (the form + * Next.js stores in history state — always the actual URL, never a route + * pattern). + * @returns NextHistoryState with the given url and no-op as/options. + */ +function buildHistoryState(url: string): NextHistoryState { + return { as: url, options: {}, url }; +} + +describe("wasPop", () => { + it("returns false when nextHistoryState is undefined", () => { + expect(wasPop("", "/projects", undefined)).toBe(false); + }); + + it("returns true when pathname matches the path component of the history URL", () => { + expect(wasPop("", "/projects", buildHistoryState("/projects"))).toBe(true); + }); + + it("strips the query string off the history URL before comparing", () => { + expect( + wasPop("", "/projects", buildHistoryState("/projects?filter=foo")), + ).toBe(true); + }); + + it("returns false when pathname does not match", () => { + expect(wasPop("", "/projects", buildHistoryState("/files"))).toBe(false); + }); + + it("defaults basePath to empty string when not provided", () => { + expect(wasPop(undefined, "/projects", buildHistoryState("/projects"))).toBe( + true, + ); + }); + + it("prepends basePath to pathname before comparing", () => { + expect( + wasPop("/data", "/projects", buildHistoryState("/data/projects")), + ).toBe(true); + }); + + it("returns false when basePath is set but missing from the history URL", () => { + expect(wasPop("/data", "/projects", buildHistoryState("/projects"))).toBe( + false, + ); + }); + + // Documents the contract the usePathname() migration relies on: pathname is + // the resolved URL (e.g. /anvil-cmg/abc-123), not the route pattern + // (/[entityListType]/[entityId]). The first matches; the second does not. + it("matches when pathname is the resolved URL (post-migration form)", () => { + expect( + wasPop( + "", + "/anvil-cmg/abc-123", + buildHistoryState("/anvil-cmg/abc-123?filter=foo"), + ), + ).toBe(true); + }); + + it("does not match when pathname is a route pattern (pre-migration form on dynamic routes)", () => { + expect( + wasPop( + "", + "/[entityListType]/[entityId]", + buildHistoryState("/anvil-cmg/abc-123"), + ), + ).toBe(false); + }); +}); + +describe("hasParams", () => { + it("returns true when any param key has a defined value in the query", () => { + expect(hasParams({ filter: "foo" }, ["filter"])).toBe(true); + }); + + it("returns true when at least one of multiple param keys is present", () => { + expect(hasParams({ sort: "asc" }, ["filter", "sort"])).toBe(true); + }); + + it("returns false when none of the param keys are in the query", () => { + expect(hasParams({ other: "x" }, ["filter", "sort"])).toBe(false); + }); + + it("returns false for an empty paramKeys list", () => { + expect(hasParams({ filter: "foo" }, [])).toBe(false); + }); + + it("returns false when a param key is present but undefined", () => { + expect(hasParams({ filter: undefined }, ["filter"])).toBe(false); + }); +}); + +describe("isSynced", () => { + it("returns true for two empty queries", () => { + expect(isSynced({}, {})).toBe(true); + }); + + it("returns true when queries have the same keys/values in different order", () => { + // eslint-disable-next-line sort-keys -- intentionally unsorted to exercise insertion-order independence. + expect(isSynced({ a: "1", b: "2" }, { b: "2", a: "1" })).toBe(true); + }); + + it("returns false when queries differ in value", () => { + expect(isSynced({ a: "1" }, { a: "2" })).toBe(false); + }); + + it("returns false when one query has extra keys", () => { + expect(isSynced({ a: "1" }, { a: "1", b: "2" })).toBe(false); + }); +}); + +describe("stringifyQuery", () => { + it("produces identical output regardless of insertion order", () => { + expect(stringifyQuery({ a: "1", b: "2" })).toBe( + // eslint-disable-next-line sort-keys -- intentionally unsorted to exercise insertion-order independence. + stringifyQuery({ b: "2", a: "1" }), + ); + }); + + it("produces empty-object JSON for an empty query", () => { + expect(stringifyQuery({})).toBe("{}"); + }); +}); From c9db476ea04644b26d5280d7076cc537c63c6102 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 26 May 2026 15:54:52 +1000 Subject: [PATCH 3/4] fix: keep useRouter().pathname in useStateSync; correct test contract (#928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NextHistoryState.url is the href stored by Next.js — the route pattern on dynamic routes — not the resolved URL, so the previous code was correctly using useRouter().pathname (also a route pattern). Migrating that site to usePathname() (resolved URL) would have broken wasPop() on dynamic routes. Reverts the useStateSync portion of the migration. Updates the unit tests to reflect the actual NextHistoryState contract. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../hooks/UseStateSync/hook.ts | 4 +-- tests/stateSyncManager_utils.test.ts | 35 +++++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts b/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts index afc241eb..393b57c5 100644 --- a/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts +++ b/src/hooks/stateSyncManager/hooks/UseStateSync/hook.ts @@ -1,4 +1,3 @@ -import { usePathname } from "next/navigation"; import { NextRouter, useRouter } from "next/router"; import { useEffect } from "react"; import { ROUTER_METHOD } from "../../../../providers/exploreState/actions/stateToUrl/types"; @@ -11,8 +10,7 @@ export const useStateSync = ({ dispatch, state, }: UseStateSyncManagerProps): void => { - const { basePath, isReady, query } = useRouter(); - const pathname = usePathname() ?? ""; + const { basePath, isReady, pathname, query } = useRouter(); const { onClearPopRef, popRef } = useWasPop(); // Extract the query from the state. diff --git a/tests/stateSyncManager_utils.test.ts b/tests/stateSyncManager_utils.test.ts index 6fe567a0..8e84ab67 100644 --- a/tests/stateSyncManager_utils.test.ts +++ b/tests/stateSyncManager_utils.test.ts @@ -8,13 +8,15 @@ import { NextHistoryState } from "../src/services/beforePopState/types"; /** * Builds a minimal NextHistoryState for tests. - * @param url - The resolved URL the browser is navigating to (the form - * Next.js stores in history state — always the actual URL, never a route - * pattern). - * @returns NextHistoryState with the given url and no-op as/options. + * @param url - The href stored by Next.js in history state (the route + * pattern on dynamic routes, e.g. `/[entityListType]/[entityId]`; the same + * string as the URL on static routes). + * @param as - The resolved URL shown to the user. Defaults to `url` for + * static-route cases where the two are identical. + * @returns NextHistoryState with the given fields and no-op options. */ -function buildHistoryState(url: string): NextHistoryState { - return { as: url, options: {}, url }; +function buildHistoryState(url: string, as: string = url): NextHistoryState { + return { as, options: {}, url }; } describe("wasPop", () => { @@ -54,25 +56,28 @@ describe("wasPop", () => { ); }); - // Documents the contract the usePathname() migration relies on: pathname is - // the resolved URL (e.g. /anvil-cmg/abc-123), not the route pattern - // (/[entityListType]/[entityId]). The first matches; the second does not. - it("matches when pathname is the resolved URL (post-migration form)", () => { + // Documents the contract: nextHistoryState.url is the href stored by + // Next.js — the route pattern on dynamic routes, the static URL on static + // routes. `pathname` must be in the same form for the comparison to match. + it("matches on a dynamic route when both sides are the route pattern", () => { expect( wasPop( "", - "/anvil-cmg/abc-123", - buildHistoryState("/anvil-cmg/abc-123?filter=foo"), + "/[entityListType]/[entityId]", + buildHistoryState( + "/[entityListType]/[entityId]?filter=foo", + "/anvil-cmg/abc-123?filter=foo", + ), ), ).toBe(true); }); - it("does not match when pathname is a route pattern (pre-migration form on dynamic routes)", () => { + it("does not match when pathname is the resolved URL but history url is the route pattern", () => { expect( wasPop( "", - "/[entityListType]/[entityId]", - buildHistoryState("/anvil-cmg/abc-123"), + "/anvil-cmg/abc-123", + buildHistoryState("/[entityListType]/[entityId]", "/anvil-cmg/abc-123"), ), ).toBe(false); }); From e365cb2bfb05596fb90a8587e100d29a850be006 Mon Sep 17 00:00:00 2001 From: Fran McDade <18710366+frano-m@users.noreply.github.com> Date: Tue, 26 May 2026 15:58:07 +1000 Subject: [PATCH 4/4] chore: use type-only import for NextHistoryState in tests (#928) Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/stateSyncManager_utils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/stateSyncManager_utils.test.ts b/tests/stateSyncManager_utils.test.ts index 8e84ab67..e0d3ee97 100644 --- a/tests/stateSyncManager_utils.test.ts +++ b/tests/stateSyncManager_utils.test.ts @@ -4,7 +4,7 @@ import { stringifyQuery, wasPop, } from "../src/hooks/stateSyncManager/hooks/UseStateSync/utils"; -import { NextHistoryState } from "../src/services/beforePopState/types"; +import type { NextHistoryState } from "../src/services/beforePopState/types"; /** * Builds a minimal NextHistoryState for tests.