From 164d95144f0c05cd208dce94e8badde0c7fc0cb3 Mon Sep 17 00:00:00 2001 From: Matt Hillsdon Date: Fri, 11 Sep 2026 16:17:02 +0000 Subject: [PATCH] Move the editor's URL state onto react-router Replaces the bespoke pushState/popstate router with react-router 7, as ml-trainer uses, ahead of adding pages beyond the editor. URLs are unchanged: the editor stays at the base URL with the documentation tab and anchor as optional path segments, and deeper paths remain the editor with no tab selected. - urls.ts holds the basename (derived from the Vite base URL) and the editor route path and link builder, used for both routes and links. - router.tsx creates the router. The root route wraps its outlet in the existing ErrorBoundary, so an uncaught render error shows the content-load-error message rather than unmounting the whole app. - router-hooks.tsx keeps the useRouterState/useRouterTabSlug API over useParams/useNavigate/useLocation. Focus travels in history state and the state identity is tied to location.key so navigating to the same anchor again re-runs scroll and focus effects, as before. - Search results use react-router's Link so the href respects the basename (the old toUrl produced a broken absolute URL). - The sidebar only resets the anchor when the clicked tab is the current one. It previously reset whenever an anchor was set and relied on the old router having updated state synchronously between pointer-up and click; with react-router that made clicking another tab from a deep link land on the current tab's top level. --- package-lock.json | 42 +++++++ package.json | 1 + src/App.tsx | 23 ++-- src/documentation/search/SearchResultList.tsx | 15 ++- src/router-hooks.test.tsx | 110 +++++++++++++++++ src/router-hooks.tsx | 114 ++++++------------ src/router.tsx | 31 +++++ src/urls.ts | 32 +++++ src/workbench/SideBar.tsx | 2 +- 9 files changed, 277 insertions(+), 93 deletions(-) create mode 100644 src/router-hooks.test.tsx create mode 100644 src/router.tsx create mode 100644 src/urls.ts diff --git a/package-lock.json b/package-lock.json index 30bf8847c..469505abf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,6 +46,7 @@ "react-hotkeys-hook": "^4.5.0", "react-icons": "^4.12.0", "react-intl": "^6.6.8", + "react-router": "^7.18.3", "vite": "^7.3.1", "vscode-jsonrpc": "^9.0.0", "vscode-languageserver-protocol": "^3.16.0", @@ -6352,6 +6353,19 @@ "dev": true, "license": "MIT" }, + "node_modules/cookie": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-1.1.1.tgz", + "integrity": "sha512-ei8Aos7ja0weRpFzJnEA9UHJ/7XQmqglbRwnf2ATjcB9Wq874VKH9kfjjirM6UhU2/E5fFYadylyhFldcqSidQ==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, "node_modules/core-js-compat": { "version": "3.50.0", "resolved": "https://registry.npmjs.org/core-js-compat/-/core-js-compat-3.50.0.tgz", @@ -10726,6 +10740,28 @@ "node": ">=0.10.0" } }, + "node_modules/react-router": { + "version": "7.18.3", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-7.18.3.tgz", + "integrity": "sha512-gyXgtdr5uACJ5b1Q4udzjVV+tb/rlHIMJKuJ0e89R4Kzgz47z/rgP0dIKxktqIEUhDHluGTPJJH/wRha7CyqsA==", + "license": "MIT", + "dependencies": { + "cookie": "^1.0.1", + "set-cookie-parser": "^2.6.0" + }, + "engines": { + "node": ">=20.0.0" + }, + "peerDependencies": { + "react": ">=18", + "react-dom": ">=18" + }, + "peerDependenciesMeta": { + "react-dom": { + "optional": true + } + } + }, "node_modules/react-stately": { "version": "3.50.0", "resolved": "https://registry.npmjs.org/react-stately/-/react-stately-3.50.0.tgz", @@ -11100,6 +11136,12 @@ "node": ">=20.0.0" } }, + "node_modules/set-cookie-parser": { + "version": "2.7.2", + "resolved": "https://registry.npmjs.org/set-cookie-parser/-/set-cookie-parser-2.7.2.tgz", + "integrity": "sha512-oeM1lpU/UvhTxw+g3cIfxXHyJRc/uidd3yK1P242gzHds0udQBYzs3y8j4gCCW+ZJ7ad0yctld8RYO+bdurlvw==", + "license": "MIT" + }, "node_modules/set-function-length": { "version": "1.2.2", "resolved": "https://registry.npmjs.org/set-function-length/-/set-function-length-1.2.2.tgz", diff --git a/package.json b/package.json index 2e91edfc5..43eb41652 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "react-hotkeys-hook": "^4.5.0", "react-icons": "^4.12.0", "react-intl": "^6.6.8", + "react-router": "^7.18.3", "vite": "^7.3.1", "vscode-jsonrpc": "^9.0.0", "vscode-languageserver-protocol": "^3.16.0", diff --git a/src/App.tsx b/src/App.tsx index e4691f06c..37a7bae50 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -5,7 +5,7 @@ */ import { SharedUIProvider, ToastProvider } from "@microbit/ui"; import { polyfill } from "mobile-drag-drop"; -import { useEffect } from "react"; +import { useEffect, useMemo } from "react"; import "./App.css"; import { DialogProvider } from "./common/use-dialogs"; import VisualViewPortCSSVariables from "./common/VisualViewportCSSVariables"; @@ -29,12 +29,12 @@ import { logDeviceStatusChange } from "./logging/analytics"; import { LoggingProvider } from "./logging/logging-hooks"; import TranslationProvider from "./messages/TranslationProvider"; import ProjectDropTarget from "./project/ProjectDropTarget"; -import { RouterProvider } from "./router-hooks"; +import { RouterProvider } from "react-router/dom"; +import { createRouter } from "./router"; import SessionSettingsProvider from "./settings/session-settings"; import SettingsProvider from "./settings/settings"; import BeforeUnloadDirtyCheck from "./workbench/BeforeUnloadDirtyCheck"; import { SelectionProvider } from "./workbench/use-selection"; -import Workbench from "./workbench/Workbench"; const isMockDeviceMode = () => // We use a cookie set from the e2e tests. Avoids having separate test and live builds. @@ -75,6 +75,7 @@ const App = () => { const deployment = useDeployment(); const { ConsentProvider } = deployment.compliance; + const router = useMemo(() => createRouter(), []); return ( <> @@ -96,15 +97,13 @@ const App = () => { - - - - - - - - - + + + + + + + diff --git a/src/documentation/search/SearchResultList.tsx b/src/documentation/search/SearchResultList.tsx index c31dd4b8f..93bae85d7 100644 --- a/src/documentation/search/SearchResultList.tsx +++ b/src/documentation/search/SearchResultList.tsx @@ -3,13 +3,20 @@ * * SPDX-License-Identifier: MIT */ -import { Divider, Link, Text } from "@microbit/ui"; +import { Divider, styled, Text } from "@microbit/ui"; import { ComponentProps } from "react"; import { FormattedMessage } from "react-intl"; +import { Link as RouterLink } from "react-router"; import { Stack } from "styled-system/jsx"; -import { RouterState, toUrl } from "../../router-hooks"; +import { link } from "styled-system/recipes"; +import { RouterState } from "../../router-hooks"; +import { createEditorUrl } from "../../urls"; import { Extract, Result } from "./common"; +// @microbit/ui's Link with react-router underneath, so the href respects the +// basename and a modifier-click opens the result in a new tab. +const Link = styled(RouterLink, link); + interface SearchResultListProps { title: string; results: Result[]; @@ -61,15 +68,13 @@ const SearchResultItem = ({ viewedResults, onViewResult, }: SearchResultItemProps) => { - const url = toUrl(navigation); - return ( { e.preventDefault(); onViewResult(id, navigation); diff --git a/src/router-hooks.test.tsx b/src/router-hooks.test.tsx new file mode 100644 index 000000000..cff888e3a --- /dev/null +++ b/src/router-hooks.test.tsx @@ -0,0 +1,110 @@ +/** + * (c) 2026, Micro:bit Educational Foundation and contributors + * + * SPDX-License-Identifier: MIT + */ +import { act, render } from "@testing-library/react"; +import { useEffect } from "react"; +import { createMemoryRouter, RouterProvider } from "react-router"; +import { LoggingProvider } from "./logging/logging-hooks"; +import { MockLogging } from "./logging/mock"; +import { NavigationSource, RouterState, useRouterState } from "./router-hooks"; +import { editorRoutePath } from "./urls"; + +const result: { current?: ReturnType } = {}; +const state = (): RouterState => result.current![0]; +const setState = (state: RouterState, source?: NavigationSource) => + result.current![1](state, source); + +const Probe = () => { + const value = useRouterState(); + useEffect(() => { + result.current = value; + }); + return null; +}; + +const renderAt = (path: string) => { + const logging = new MockLogging(); + const router = createMemoryRouter( + [ + { + path: "", + children: [ + { path: editorRoutePath, element: }, + { path: "*", element: }, + ], + }, + ], + { initialEntries: [path] } + ); + render( + + + + ); + return { router, logging }; +}; + +describe("useRouterState", () => { + it("is empty at the root", () => { + renderAt("/"); + expect(state()).toEqual({}); + }); + + it("reads the tab and slug from the path", () => { + renderAt("/reference/display"); + expect(state()).toEqual({ + tab: "reference", + slug: { id: "display" }, + focus: false, + }); + }); + + it("ignores unknown tabs", () => { + renderAt("/nonsense/display"); + expect(state()).toEqual({}); + }); + + it("treats deeper paths as the editor with no tab", () => { + renderAt("/api/a/b"); + expect(state()).toEqual({}); + }); + + it("navigates, carrying focus in history state, and logs the source", async () => { + const { router, logging } = renderAt("/"); + await act(async () => { + setState({ tab: "api", slug: { id: "microbit" }, focus: true }, "code"); + }); + expect(router.state.location.pathname).toEqual("/api/microbit"); + expect(state()).toEqual({ + tab: "api", + slug: { id: "microbit" }, + focus: true, + }); + expect(logging.events).toEqual([ + { + type: "docs_navigate", + detail: { via: "code", surface: "api", id: "microbit" }, + }, + ]); + }); + + it("does not log without a source", async () => { + const { logging } = renderAt("/"); + await act(async () => { + setState({ tab: "ideas" }); + }); + expect(logging.events).toEqual([]); + }); + + it("gives a new state object when navigating to the same anchor again", async () => { + renderAt("/reference/display"); + const before = state(); + await act(async () => { + setState({ tab: "reference", slug: { id: "display" } }); + }); + expect(state()).not.toBe(before); + expect(state()).toEqual(before); + }); +}); diff --git a/src/router-hooks.tsx b/src/router-hooks.tsx index f9e7d1dc2..aa728b435 100644 --- a/src/router-hooks.tsx +++ b/src/router-hooks.tsx @@ -1,29 +1,26 @@ /** - * A simple custom router. We don't have pages as such, but the different UI areas - * use query parameters to keep some state, for example to allow navigating back from - * drilling down into the documentation in the side panel or making tab selections. + * Editor URL state. * - * Which UI state is encoded into the URL might be subject to change in future - * based on user feedback and discussion. + * The documentation tabs and their drill-down anchors are encoded in the URL + * so that browser navigation works for them. These hooks are the app's view + * of that state over react-router. * - * (c) 2021-2022, Micro:bit Educational Foundation and contributors + * (c) 2021-2026, Micro:bit Educational Foundation and contributors * * SPDX-License-Identifier: MIT */ -import { - createContext, - ReactNode, - useCallback, - useContext, - useEffect, - useMemo, - useState, -} from "react"; -import { baseUrl } from "./base"; +import { useCallback, useMemo } from "react"; +import { useLocation, useNavigate, useParams } from "react-router"; import { useLogging } from "./logging/logging-hooks"; +import { createEditorUrl } from "./urls"; export type TabName = "api" | "ideas" | "reference" | "project"; +const tabNames: readonly string[] = ["api", "ideas", "reference", "project"]; + +const isTabName = (value: string | undefined): value is TabName => + value !== undefined && tabNames.includes(value); + /** * An anchor-like navigation used for scroll positions. * @@ -33,8 +30,6 @@ export type TabName = "api" | "ideas" | "reference" | "project"; export interface Anchor { id: string; } -const anchorForParam = (param: string | null): Anchor | undefined => - param ? { id: param } : undefined; export interface RouterState { tab?: TabName; @@ -52,63 +47,38 @@ type RouterContextValue = [ (state: RouterState, source?: NavigationSource) => void ]; -const RouterContext = createContext(undefined); - -const parse = (pathname: string): RouterState => { - pathname = pathname.slice(baseUrl.length); - if (pathname) { - const parts = pathname.split("/"); - const tab = parts[0]; - if ( - tab === "api" || - tab === "reference" || - tab === "ideas" || - tab === "project" - ) { - return { tab, slug: anchorForParam(parts[1]) }; - } - } - return {}; -}; +/** Carried in history state rather than the URL. */ +interface LocationState { + focus?: boolean; +} /** * The full router state. - * Consider using useRouterParam instead if you only care about one parameter. + * Consider using useRouterTabSlug instead if you only care about one parameter. * * Updating the state updates the URL. * * @return a [state, setState] pair. */ export const useRouterState = (): RouterContextValue => { - const value = useContext(RouterContext); - if (!value) { - throw new Error("Missing provider!"); - } - return value; -}; + const { tab, slug } = useParams<"tab" | "slug">(); + const location = useLocation(); + const navigate = useNavigate(); + const logging = useLogging(); -export const toUrl = (state: RouterState): string => { - const parts = [state.tab, state.slug?.id]; - const pathname = baseUrl + parts.filter((x): x is string => !!x).join("/"); - return window.location.toString().split("/", 1)[0] + pathname; -}; + const focus = (location.state as LocationState | null)?.focus ?? false; + const state = useMemo( + () => + isTabName(tab) + ? { tab, slug: slug ? { id: slug } : undefined, focus } + : {}, + // location.key: navigating to the current anchor again must produce a new + // state object so that the scroll and focus effects run again. + // eslint-disable-next-line react-hooks/exhaustive-deps + [tab, slug, focus, location.key] + ); -export const RouterProvider = ({ children }: { children: ReactNode }) => { - const logging = useLogging(); - const [state, setState] = useState(() => parse(window.location.pathname)); - useEffect(() => { - // This detects browser navigation but not our programatic changes, - // so we need to update state there ourselves. - const listener = (_: PopStateEvent) => { - const newState = parse(window.location.pathname); - setState(newState); - }; - window.addEventListener("popstate", listener); - return () => { - window.removeEventListener("popstate", listener); - }; - }, [setState]); - const navigate = useCallback( + const setState = useCallback( (newState: RouterState, source?: NavigationSource) => { if (source) { logging.event({ @@ -116,19 +86,13 @@ export const RouterProvider = ({ children }: { children: ReactNode }) => { detail: { via: source, surface: newState.tab, id: newState.slug?.id }, }); } - const url = toUrl(newState); - window.history.pushState(newState, "", url); - - setState(newState); + const locationState: LocationState = { focus: newState.focus }; + void navigate(createEditorUrl(newState), { state: locationState }); }, - [logging, setState] - ); - const value: RouterContextValue = useMemo(() => { - return [state, navigate]; - }, [state, navigate]); - return ( - {children} + [logging, navigate] ); + + return useMemo(() => [state, setState], [state, setState]); }; /** diff --git a/src/router.tsx b/src/router.tsx new file mode 100644 index 000000000..e564fdc53 --- /dev/null +++ b/src/router.tsx @@ -0,0 +1,31 @@ +/** + * (c) 2026, Micro:bit Educational Foundation and contributors + * + * SPDX-License-Identifier: MIT + */ +import { createBrowserRouter, Outlet } from "react-router"; +import ErrorBoundary from "./common/ErrorBoundary"; +import { basename, editorRoutePath } from "./urls"; +import Workbench from "./workbench/Workbench"; + +export const createRouter = () => + createBrowserRouter( + [ + { + id: "root", + path: "", + // Without this an uncaught render error unmounts the whole app. + element: ( + + + + ), + children: [ + { path: editorRoutePath, element: }, + // Deeper paths are the editor with no tab selected, as before. + { path: "*", element: }, + ], + }, + ], + { basename } + ); diff --git a/src/urls.ts b/src/urls.ts new file mode 100644 index 000000000..166405198 --- /dev/null +++ b/src/urls.ts @@ -0,0 +1,32 @@ +/** + * App URLs. + * + * Paths are relative to the router basename: react-router adds it when + * navigating or rendering a Link. Do not use them with window.location or a + * plain anchor. + * + * (c) 2026, Micro:bit Educational Foundation and contributors + * + * SPDX-License-Identifier: MIT + */ +import { baseUrl } from "./base"; +import type { RouterState } from "./router-hooks"; + +/** + * The react-router basename: the Vite base URL without its trailing slash, so + * that both `/v/3` and `/v/3/` are the editor. + */ +export const basename = + baseUrl === "/" ? undefined : baseUrl.replace(/\/$/, ""); + +/** + * Route path for the editor. The documentation tab and its anchor are + * optional segments so the editor stays mounted as they change. + */ +export const editorRoutePath = ":tab?/:slug?"; + +/** + * Path for the editor showing the given documentation tab and anchor. + */ +export const createEditorUrl = ({ tab, slug }: RouterState = {}): string => + "/" + [tab, slug?.id].filter((x): x is string => !!x).join("/"); diff --git a/src/workbench/SideBar.tsx b/src/workbench/SideBar.tsx index d2a73bd41..693a10c28 100644 --- a/src/workbench/SideBar.tsx +++ b/src/workbench/SideBar.tsx @@ -160,7 +160,7 @@ const SideBar = ({ // previous selection is retained (but not styled). Clicking it // fires no selection change, so expand from the click instead. handleTabChange(panes.findIndex((p) => p.id === id)); - } else if (slug) { + } else if (id === tab && slug) { // A click on a tab when it's already selected should // reset any other parameters so we go back to the top // level.