Rationalise URLs (#970)

The previous scheme assumed that we'd maintain independent state for the tabs, but we decided against that. So now we can go for a more traditional URL routing scheme.

Author: microbit-matt-hillsdon

src/base.ts

@@ -0,0 +1,7 @@
+export const baseUrl = (() => {
+  let base = process.env.PUBLIC_URL || "/";
+  if (!base.endsWith("/")) {
+    base += "/";
+  }
+  return base;
+})();

src/documentation/api/ApiDocumentation.tsx

@@ -13,12 +13,7 @@ import { docStyles } from "../../common/documentation-styles";
 import HeadedScrollablePanel from "../../common/HeadedScrollablePanel";
 import { splitDocString } from "../../editor/codemirror/language-server/docstrings";
 import { ApiDocsEntry, ApiDocsResponse } from "../../language-server/apidocs";
-import {
-  Anchor,
-  RouterParam,
-  useRouterParam,
-  useRouterState,
-} from "../../router-hooks";
+import { Anchor, useRouterTabSlug, useRouterState } from "../../router-hooks";
 import DocString from "../common/DocString";
 import { useAnimationDirection } from "../common/documentation-animation-hooks";
 import DocumentationBreadcrumbHeading from "../common/DocumentationBreadcrumbHeading";
@@ -32,7 +27,7 @@ interface ApiDocumentationProps {
 }
 
 export const ApiDocumentation = ({ docs }: ApiDocumentationProps) => {
-  const [anchor, setAnchor] = useRouterParam(RouterParam.api);
+  const [anchor, setAnchor] = useRouterTabSlug("api");
   const handleNavigate = useCallback(
     (id: string | undefined) => {
       setAnchor(id ? { id } : undefined, "documentation-user");

src/documentation/api/apidocs-util.test.ts

@@ -4,11 +4,7 @@
  * SPDX-License-Identifier: MIT
  */
 import { ApiDocsResponse } from "../../language-server/apidocs";
-import {
-  moduleAndApiFromId,
-  pullModulesToTop,
-  resolveDottedName,
-} from "./apidocs-util";
+import { moduleAndApiFromId, pullModulesToTop } from "./apidocs-util";
 
 describe("pullModulesToTop", () => {
   it("pulls modules up", () => {
@@ -72,68 +68,6 @@ describe("pullModulesToTop", () => {
   });
 });
 
-describe("resolveDottedName", () => {
-  const docs: ApiDocsResponse = {
-    microbit: {
-      fullName: "microbit",
-      id: "microbit",
-      name: "microbit",
-      kind: "module",
-      children: [
-        {
-          fullName: "microbit.Button",
-          name: "Button",
-          kind: "class",
-          id: "microbit.Button",
-          children: [
-            {
-              fullName: "microbit.Button.is_pressed",
-              kind: "function",
-              id: "microbit.Button.is_pressed",
-              name: "is_pressed",
-            },
-          ],
-        },
-      ],
-    },
-    "microbit.compass": {
-      fullName: "microbit.compass",
-      id: "microbit.compass",
-      name: "compass",
-      kind: "module",
-      children: [
-        {
-          fullName: "microbit.compass.get_x",
-          kind: "function",
-          id: "microbit.compass.get_x",
-          name: "get_x",
-        },
-      ],
-    },
-  };
-  it("finds modules", () => {
-    expect(resolveDottedName(docs, "microbit.compass")).toEqual(
-      docs["microbit.compass"]
-    );
-    expect(resolveDottedName(docs, "microbit")).toEqual(docs["microbit"]);
-  });
-  it("finds classes", () => {
-    expect(resolveDottedName(docs, "microbit.Button")?.fullName).toEqual(
-      "microbit.Button"
-    );
-  });
-  it("finds functions", () => {
-    expect(resolveDottedName(docs, "microbit.compass.get_x")?.fullName).toEqual(
-      "microbit.compass.get_x"
-    );
-  });
-  it("finds class members", () => {
-    expect(
-      resolveDottedName(docs, "microbit.Button.is_pressed")?.fullName
-    ).toEqual("microbit.Button.is_pressed");
-  });
-});
-
 describe("moduleAndApiFromId", () => {
   it("correctly splits module and apiId from id with three segments", () => {
     const id = "microbit.display.scroll";

src/documentation/api/apidocs-util.ts

@@ -39,24 +39,6 @@ export const resolveModule = (
     );
 };
 
-export const resolveDottedName = (docs: ApiDocsResponse, name: string) => {
-  let entry = resolveModule(docs, name);
-  if (!entry) {
-    return undefined;
-  }
-  const remainder = name.substring(entry.fullName.length + 1);
-  if (remainder.length > 0) {
-    for (const part of remainder.split(".")) {
-      if (entry && entry.children) {
-        entry = entry.children.find((e) => e.name === part);
-      } else {
-        return undefined;
-      }
-    }
-  }
-  return entry;
-};
-
 export const moduleAndApiFromId = (id: string) => {
   const idSegments = id.split(".");
   const pythonModuleName = idSegments[0];

src/documentation/common/DocumentationContent.tsx

@@ -74,7 +74,7 @@ const DocumentationApiLinkMark = (
         e.preventDefault();
         setState({
           tab: "api",
-          api: { id: props.mark.name },
+          slug: { id: props.mark.name },
         });
       }}
     >
@@ -96,7 +96,7 @@ const DocumentationInternalLinkMark = (
           {
             ...state,
             tab: "reference",
-            reference: {
+            slug: {
               id: props.mark.slug.current,
             },
           },

src/documentation/ideas/IdeasDocumentation.tsx

@@ -12,7 +12,7 @@ import { docStyles } from "../../common/documentation-styles";
 import HeadedScrollablePanel from "../../common/HeadedScrollablePanel";
 import { getAspectRatio, imageUrlBuilder } from "../../common/imageUrlBuilder";
 import { useResizeObserverContentRect } from "../../common/use-resize-observer";
-import { Anchor, RouterParam, useRouterParam } from "../../router-hooks";
+import { Anchor, useRouterTabSlug } from "../../router-hooks";
 import { useAnimationDirection } from "../common/documentation-animation-hooks";
 import DocumentationBreadcrumbHeading from "../common/DocumentationBreadcrumbHeading";
 import DocumentationContent, {
@@ -27,7 +27,7 @@ interface IdeasDocumentationProps {
 }
 
 const IdeasDocumentation = ({ ideas }: IdeasDocumentationProps) => {
-  const [anchor, setAnchor] = useRouterParam(RouterParam.idea);
+  const [anchor, setAnchor] = useRouterTabSlug("ideas");
   const direction = useAnimationDirection(anchor);
   const ideaId = anchor?.id;
   const handleNavigate = useCallback(

src/documentation/reference/ReferenceDocumentation.tsx

@@ -8,7 +8,7 @@ import { useCallback } from "react";
 import { useIntl } from "react-intl";
 import { docStyles } from "../../common/documentation-styles";
 import HeadedScrollablePanel from "../../common/HeadedScrollablePanel";
-import { Anchor, RouterParam, useRouterParam } from "../../router-hooks";
+import { Anchor, useRouterTabSlug } from "../../router-hooks";
 import { useAnimationDirection } from "../common/documentation-animation-hooks";
 import DocumentationBreadcrumbHeading from "../common/DocumentationBreadcrumbHeading";
 import DocumentationContent from "../common/DocumentationContent";
@@ -29,7 +29,7 @@ interface ReferenceDocumentationProps {
  * generate the API documentation.
  */
 const ReferenceToolkit = ({ toolkit }: ReferenceDocumentationProps) => {
-  const [anchor, setAnchor] = useRouterParam(RouterParam.reference);
+  const [anchor, setAnchor] = useRouterTabSlug("reference");
   const direction = useAnimationDirection(anchor);
   const topicOrEntryId = anchor?.id.split("/")[0];
   const handleNavigate = useCallback(

src/documentation/search/search.test.ts

@@ -65,7 +65,7 @@ describe("Search", () => {
         id: "indentations",
         navigation: {
           tab: "reference",
-          reference: { id: "indentations" },
+          slug: { id: "indentations" },
         },
         extract: {
           title: [

src/documentation/search/search.ts

@@ -93,7 +93,7 @@ export class SearchIndex {
         containerTitle: content.containerTitle,
         navigation: {
           tab: this.tab,
-          [this.tab]: { id: content.id },
+          slug: { id: content.id },
         },
         extract: extracts,
       };

src/editor/codemirror/CodeMirror.tsx

@@ -240,7 +240,7 @@ const CodeMirror = ({
       setRouterState(
         {
           tab,
-          [tab]: { id },
+          slug: { id },
         },
         "documentation-from-code"
       );

src/editor/codemirror/language-server/documentation.ts

@@ -159,6 +159,7 @@ export const wrapWithDocumentationButton = (
     apiAnchor.textContent = intl.formatMessage({ id: "api-tab" });
     apiAnchor.onclick = (e) => {
       e.preventDefault();
+      // Could we instead interact with the history API here?
       document.dispatchEvent(
         new CustomEvent("cm/openDocs", {
           detail: {

src/language-server/pyright.ts

@@ -8,6 +8,7 @@ import {
   BrowserMessageReader,
   BrowserMessageWriter,
 } from "vscode-jsonrpc/browser";
+import { baseUrl } from "../base";
 import { createUri, LanguageServerClient } from "./client";
 
 // This is modified by bin/update-pyright.sh
@@ -24,9 +25,7 @@ export const pyright = (language: string): LanguageServerClient | undefined => {
     return undefined;
   }
   // Needed to support review branches that use a path location.
-  const { origin, pathname } = window.location;
-  const base = `${origin}${pathname}${pathname.endsWith("/") ? "" : "/"}`;
-  const workerScript = `${base}workers/${workerScriptName}`;
+  const workerScript = `${baseUrl}workers/${workerScriptName}`;
   const foreground = new Worker(workerScript, {
     name: "Pyright-foreground",
   });

src/router-hooks.tsx

@@ -19,8 +19,11 @@ import {
   useMemo,
   useState,
 } from "react";
+import { baseUrl } from "./base";
 import { useLogging } from "./logging/logging-hooks";
 
+export type TabName = "api" | "ideas" | "reference" | "project";
+
 /**
  * An anchor-like navigation used for scroll positions.
  *
@@ -33,25 +36,9 @@ export interface Anchor {
 const anchorForParam = (param: string | null): Anchor | undefined =>
   param ? { id: param } : undefined;
 
-export class RouterParam<T> {
-  static tab: RouterParam<string> = new RouterParam("tab");
-  static api: RouterParam<Anchor> = new RouterParam("api");
-  static reference: RouterParam<Anchor> = new RouterParam("reference");
-  static idea: RouterParam<Anchor> = new RouterParam("idea");
-
-  private constructor(public id: keyof RouterState) {}
-
-  get(state: RouterState): T | undefined {
-    // Constructor is private so this is safe.
-    return state[this.id] as unknown as T | undefined;
-  }
-}
-
 export interface RouterState {
-  tab?: string;
-  reference?: Anchor;
-  api?: Anchor;
-  idea?: Anchor;
+  tab?: TabName;
+  slug?: Anchor;
   focus?: boolean;
 }
 
@@ -68,14 +55,21 @@ type RouterContextValue = [
 
 const RouterContext = createContext<RouterContextValue | undefined>(undefined);
 
-const parse = (search: string): RouterState => {
-  const params = new URLSearchParams(search);
-  return {
-    tab: params.get("tab") ?? undefined,
-    api: anchorForParam(params.get("api")),
-    reference: anchorForParam(params.get("reference")),
-    idea: anchorForParam(params.get("idea")),
-  };
+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 {};
 };
 
 /**
@@ -95,28 +89,19 @@ export const useRouterState = (): RouterContextValue => {
 };
 
 export const toUrl = (state: RouterState): string => {
-  const query = Object.entries(state)
-    .filter(([k, v]) => k !== "focus" && !!v)
-    .map(([k, v]) => {
-      return `${encodeURIComponent(k)}=${encodeURIComponent(
-        serializeValue(v)
-      )}`;
-    })
-    .join("&");
-  return window.location.toString().split("?")[0] + (query ? "?" + query : "");
+  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 serializeValue = (value: Anchor | string) =>
-  typeof value === "string" ? value : value.id;
-
 export const RouterProvider = ({ children }: { children: ReactNode }) => {
   const logging = useLogging();
-  const [state, setState] = useState(parse(window.location.search));
+  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.search);
+      const newState = parse(window.location.pathname);
       setState(newState);
     };
     window.addEventListener("popstate", listener);
@@ -129,10 +114,7 @@ export const RouterProvider = ({ children }: { children: ReactNode }) => {
       if (source) {
         logging.event({
           type: source,
-          message:
-            (newState.reference?.id && `reference-${newState.reference?.id}`) ||
-            (newState.api?.id && `api-${newState.api?.id}`) ||
-            (newState.idea?.id && `ideas-${newState.idea?.id}`),
+          message: toUrl(newState),
         });
       }
       const url = toUrl(newState);
@@ -151,24 +133,23 @@ export const RouterProvider = ({ children }: { children: ReactNode }) => {
 };
 
 /**
- * Access a single parameter of the router state.
- * All other parameters will remain unchanged if you set this parameter.
+ * Access the slug for a particular tab.
  *
- * @param param The parameter name.
- * @returns A [state, setState] pair for the parameter.
+ * @param tab The tab name.
+ * @returns A [state, setState] pair for the tab.
  */
-export const useRouterParam = <T,>(
-  param: RouterParam<T>
+export const useRouterTabSlug = (
+  tab: TabName
 ): [
-  T | undefined,
-  (param: T | undefined, source?: NavigationSource) => void
+  Anchor | undefined,
+  (param: Anchor | undefined, source?: NavigationSource) => void
 ] => {
   const [state, setState] = useRouterState();
   const navigateParam = useCallback(
-    (value: T | undefined, source?: NavigationSource) => {
-      setState({ ...state, [param.id]: value }, source);
+    (value: Anchor | undefined, source?: NavigationSource) => {
+      setState({ ...state, tab, slug: value }, source);
     },
-    [param, setState, state]
+    [tab, setState, state]
   );
-  return [param.get(state), navigateParam];
+  return [state.tab === tab ? state.slug : undefined, navigateParam];
 };