Small refactor of URLParams stuff

Splits out the room locartion parsing from everything else to avoid
one function that fills out different parts of its return struct
depending on its args.

src/UrlParams.ts

@@ -21,10 +21,21 @@ import { Config } from "./config/Config";
 
 export const PASSWORD_STRING = "password=";
 
-interface UrlParams {
+interface RoomIdentifier {
   roomAlias: string | null;
   roomId: string | null;
   viaServers: string[];
+}
+
+interface UrlParams {
+  /**
+   * Anything about what room we're pointed to should be from useRoomIdentifier which
+   * parses the path and resolves alias with respect to the default server name, however
+   * roomId is an exception as we need the room ID in embedded (matroyska) mode, and not
+   * the room alias (or even the via params because we are not trying to join it). This
+   * is also not validated, where it is in useRoomIdentifier().
+   */
+  roomId: string | null;
   /**
    * Whether the app is running in embedded mode, and should keep the user
    * confined to the current room.
@@ -94,25 +105,93 @@ interface UrlParams {
   password: string | null;
 }
 
+class ParamParser {
+  private fragmentParams: URLSearchParams;
+  private queryParams: URLSearchParams;
+
+  constructor(search: string, hash: string) {
+    this.queryParams = new URLSearchParams(search);
+
+    const fragmentQueryStart = hash.indexOf("?");
+    this.fragmentParams = new URLSearchParams(
+      fragmentQueryStart === -1 ? "" : hash.substring(fragmentQueryStart)
+    );
+  }
+
+  // Normally, URL params should be encoded in the fragment so as to avoid
+  // leaking them to the server. However, we also check the normal query
+  // string for backwards compatibility with versions that only used that.
+  hasParam(name: string): boolean {
+    return this.fragmentParams.has(name) || this.queryParams.has(name);
+  }
+
+  getParam(name: string): string | null {
+    return this.fragmentParams.get(name) ?? this.queryParams.get(name);
+  }
+
+  getAllParams(name: string): string[] {
+    return [
+      ...this.fragmentParams.getAll(name),
+      ...this.queryParams.getAll(name),
+    ];
+  }
+}
+
 /**
  * Gets the app parameters for the current URL.
- * @param ignoreRoomAlias If true, does not try to parse a room alias from the URL
  * @param search The URL search string
- * @param pathname The URL path name
  * @param hash The URL hash
  * @returns The app parameters encoded in the URL
  */
 export const getUrlParams = (
-  ignoreRoomAlias?: boolean,
   search = window.location.search,
-  pathname = window.location.pathname,
   hash = window.location.hash
 ): UrlParams => {
-  // This is legacy code - we're moving away from using aliases
+  const parser = new ParamParser(search, hash);
+
+  const fontScale = parseFloat(parser.getParam("fontScale") ?? "");
+
+  return {
+    // NB. we don't validate roomId here as we do in getRoomIdentifierFromUrl:
+    // what would we do if it were invalid? If the widget API says that's what
+    // the room ID is, then that's what it is.
+    roomId: parser.getParam("roomId"),
+    password: parser.getParam("password"),
+    isEmbedded: parser.hasParam("embed"),
+    preload: parser.hasParam("preload"),
+    hideHeader: parser.hasParam("hideHeader"),
+    hideScreensharing: parser.hasParam("hideScreensharing"),
+    isPtt: parser.hasParam("ptt"),
+    e2eEnabled: parser.getParam("enableE2e") !== "false", // Defaults to true
+    userId: parser.getParam("userId"),
+    displayName: parser.getParam("displayName"),
+    deviceId: parser.getParam("deviceId"),
+    baseUrl: parser.getParam("baseUrl"),
+    lang: parser.getParam("lang"),
+    fonts: parser.getAllParams("font"),
+    fontScale: Number.isNaN(fontScale) ? null : fontScale,
+    analyticsID: parser.getParam("analyticsID"),
+    allowIceFallback: parser.hasParam("allowIceFallback"),
+  };
+};
+
+/**
+ * Hook to simplify use of getUrlParams.
+ * @returns The app parameters for the current URL
+ */
+export const useUrlParams = (): UrlParams => {
+  const { search, hash } = useLocation();
+  return useMemo(() => getUrlParams(search, hash), [search, hash]);
+};
+
+export function getRoomIdentifierFromUrl(
+  pathname: string,
+  search: string,
+  hash: string
+): RoomIdentifier {
   let roomAlias: string | null = null;
-  if (!ignoreRoomAlias) {
+
-    // Here we handle the beginning of the alias and make sure it starts with a
+  // Here we handle the beginning of the alias and make sure it starts with a "#"
-    // "#"
   if (hash === "" || hash.startsWith("#?")) {
     roomAlias = pathname.substring(1); // Strip the "/"
 
@@ -140,30 +219,11 @@ export const getUrlParams = (
       roomAlias = `${roomAlias}:${Config.defaultServerName()}`;
     }
   }
-  }
 
-  const fragmentQueryStart = hash.indexOf("?");
+  const parser = new ParamParser(search, hash);
-  const fragmentParams = new URLSearchParams(
-    fragmentQueryStart === -1 ? "" : hash.substring(fragmentQueryStart)
-  );
-  const queryParams = new URLSearchParams(search);
-
-  // Normally, URL params should be encoded in the fragment so as to avoid
-  // leaking them to the server. However, we also check the normal query
-  // string for backwards compatibility with versions that only used that.
-  const hasParam = (name: string): boolean =>
-    fragmentParams.has(name) || queryParams.has(name);
-  const getParam = (name: string): string | null =>
-    fragmentParams.get(name) ?? queryParams.get(name);
-  const getAllParams = (name: string): string[] => [
-    ...fragmentParams.getAll(name),
-    ...queryParams.getAll(name),
-  ];
-
-  const fontScale = parseFloat(getParam("fontScale") ?? "");
 
   // Make sure roomId is valid
-  let roomId: string | null = getParam("roomId");
+  let roomId: string | null = parser.getParam("roomId");
   if (!roomId?.startsWith("!")) {
     roomId = null;
   } else if (!roomId.includes("")) {
@@ -173,34 +233,14 @@ export const getUrlParams = (
   return {
     roomAlias,
     roomId,
-    password: getParam("password"),
+    viaServers: parser.getAllParams("viaServers"),
-    viaServers: getAllParams("via"),
-    isEmbedded: hasParam("embed"),
-    preload: hasParam("preload"),
-    hideHeader: hasParam("hideHeader"),
-    hideScreensharing: hasParam("hideScreensharing"),
-    isPtt: hasParam("ptt"),
-    e2eEnabled: getParam("enableE2e") !== "false", // Defaults to true
-    userId: getParam("userId"),
-    displayName: getParam("displayName"),
-    deviceId: getParam("deviceId"),
-    baseUrl: getParam("baseUrl"),
-    lang: getParam("lang"),
-    fonts: getAllParams("font"),
-    fontScale: Number.isNaN(fontScale) ? null : fontScale,
-    analyticsID: getParam("analyticsID"),
-    allowIceFallback: hasParam("allowIceFallback"),
-  };
   };
+}
 
-/**
+export const useRoomIdentifier = (): RoomIdentifier => {
- * Hook to simplify use of getUrlParams.
+  const { pathname, search, hash } = useLocation();
- * @returns The app parameters for the current URL
- */
-export const useUrlParams = (): UrlParams => {
-  const { search, pathname, hash } = useLocation();
   return useMemo(
-    () => getUrlParams(false, search, pathname, hash),
+    () => getRoomIdentifierFromUrl(pathname, search, hash),
-    [search, pathname, hash]
+    [pathname, search, hash]
   );
 };

src/initializer.tsx

@@ -61,7 +61,7 @@ export class Initializer {
     languageDetector.addDetector({
       name: "urlFragment",
       // Look for a language code in the URL's fragment
-      lookup: () => getUrlParams(true).lang ?? undefined,
+      lookup: () => getUrlParams().lang ?? undefined,
     });
 
     i18n
@@ -94,7 +94,7 @@ export class Initializer {
     }
 
     // Custom fonts
-    const { fonts, fontScale } = getUrlParams(true);
+    const { fonts, fontScale } = getUrlParams();
     if (fontScale !== null) {
       document.documentElement.style.setProperty(
         "--font-scale",

src/room/RoomPage.tsx

@@ -22,22 +22,17 @@ import { ErrorView, LoadingView } from "../FullScreenView";
 import { RoomAuthView } from "./RoomAuthView";
 import { GroupCallLoader } from "./GroupCallLoader";
 import { GroupCallView } from "./GroupCallView";
-import { useUrlParams } from "../UrlParams";
+import { useRoomIdentifier, useUrlParams } from "../UrlParams";
 import { useRegisterPasswordlessUser } from "../auth/useRegisterPasswordlessUser";
 import { useOptInAnalytics } from "../settings/useSetting";
 import { HomePage } from "../home/HomePage";
 
 export const RoomPage: FC = () => {
-  const {
+  const { isEmbedded, preload, hideHeader, isPtt, displayName } =
-    roomAlias,
+    useUrlParams();
-    roomId,
+
-    viaServers,
+  const { roomAlias, roomId, viaServers } = useRoomIdentifier();
-    isEmbedded,
+
-    preload,
-    hideHeader,
-    isPtt,
-    displayName,
-  } = useUrlParams();
   const roomIdOrAlias = roomId ?? roomAlias;
   if (!roomIdOrAlias) {
     console.error("No room specified");

src/widget.ts

@@ -109,7 +109,7 @@ export const widget: WidgetHelpers | null = (() => {
         baseUrl,
         e2eEnabled,
         allowIceFallback,
-      } = getUrlParams(true);
+      } = getUrlParams();
       if (!roomId) throw new Error("Room ID must be supplied");
       if (!userId) throw new Error("User ID must be supplied");
       if (!deviceId) throw new Error("Device ID must be supplied");

test/UrlParams-test.ts

@@ -15,7 +15,8 @@ limitations under the License.
 */
 
 import { mocked } from "jest-mock";
-import { getUrlParams } from "../src/UrlParams";
+
+import { getRoomIdentifierFromUrl } from "../src/UrlParams";
 import { Config } from "../src/config/Config";
 
 const ROOM_NAME = "roomNameHere";
@@ -32,27 +33,28 @@ describe("UrlParams", () => {
 
   describe("handles URL with /room/", () => {
     it("and nothing else", () => {
-      expect(getUrlParams(false, "", `/room/${ROOM_NAME}`, "").roomAlias).toBe(
+      expect(
-        `#${ROOM_NAME}:${HOMESERVER}`
+        getRoomIdentifierFromUrl(`/room/${ROOM_NAME}`, "", "").roomAlias
-      );
+      ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and #", () => {
       expect(
-        getUrlParams(false, "", `${ORIGIN}/room/`, `#${ROOM_NAME}`).roomAlias
+        getRoomIdentifierFromUrl("", `${ORIGIN}/room/`, `#${ROOM_NAME}`)
+          .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and # and server part", () => {
       expect(
-        getUrlParams(false, "", `/room/`, `#${ROOM_NAME}:${HOMESERVER}`)
+        getRoomIdentifierFromUrl("", `/room/`, `#${ROOM_NAME}:${HOMESERVER}`)
           .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and server part", () => {
       expect(
-        getUrlParams(false, "", `/room/${ROOM_NAME}:${HOMESERVER}`, "")
+        getRoomIdentifierFromUrl(`/room/${ROOM_NAME}:${HOMESERVER}`, "", "")
           .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
@@ -60,39 +62,44 @@ describe("UrlParams", () => {
 
   describe("handles URL without /room/", () => {
     it("and nothing else", () => {
-      expect(getUrlParams(false, "", `/${ROOM_NAME}`, "").roomAlias).toBe(
+      expect(getRoomIdentifierFromUrl(`/${ROOM_NAME}`, "", "").roomAlias).toBe(
         `#${ROOM_NAME}:${HOMESERVER}`
       );
     });
 
     it("and with #", () => {
-      expect(getUrlParams(false, "", "", `#${ROOM_NAME}`).roomAlias).toBe(
+      expect(getRoomIdentifierFromUrl("", "", `#${ROOM_NAME}`).roomAlias).toBe(
         `#${ROOM_NAME}:${HOMESERVER}`
       );
     });
 
     it("and with # and server part", () => {
       expect(
-        getUrlParams(false, "", "", `#${ROOM_NAME}:${HOMESERVER}`).roomAlias
+        getRoomIdentifierFromUrl("", "", `#${ROOM_NAME}:${HOMESERVER}`)
+          .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
 
     it("and with server part", () => {
       expect(
-        getUrlParams(false, "", `/${ROOM_NAME}:${HOMESERVER}`, "").roomAlias
+        getRoomIdentifierFromUrl(`/${ROOM_NAME}:${HOMESERVER}`, "", "")
+          .roomAlias
       ).toBe(`#${ROOM_NAME}:${HOMESERVER}`);
     });
   });
 
   describe("handles search params", () => {
     it("(roomId)", () => {
-      expect(getUrlParams(true, `?roomId=${ROOM_ID}`).roomId).toBe(ROOM_ID);
+      expect(
+        getRoomIdentifierFromUrl("", `?roomId=${ROOM_ID}`, "").roomId
+      ).toBe(ROOM_ID);
     });
   });
 
   it("ignores room alias", () => {
     expect(
-      getUrlParams(true, "", `/room/${ROOM_NAME}:${HOMESERVER}`).roomAlias
+      getRoomIdentifierFromUrl("", `/room/${ROOM_NAME}:${HOMESERVER}`, "")
+        .roomAlias
     ).toBeFalsy();
   });
 });