[FIX] router: add support for more arbitrary param placements in paths

Before this commit, params in paths had to be be between slashes and
comprise the entirety of the contents between those slashes (eg:
`/books/{{id}}/{{name}}`)

This is unnecessarily restrictive. This commit removes this restriction,
which allows for paths such as:
- `/books/{{id}}-{{name}}`
- `#books&id={{id}}&name={{name}}`

among others

closes #858

src/router/router.ts

@@ -11,6 +11,7 @@ type NavigationGuard = (info: {
 export interface Route {
   name: string;
   path: string;
+  extractionRegExp: RegExp;
   component?: any;
   redirect?: Destination;
   params: string[];
@@ -54,6 +55,7 @@ export interface EnvWithRouter extends Env {
 }
 
 const paramRegexp = /\{\{(.*?)\}\}/;
+const globalParamRegexp = new RegExp(paramRegexp.source, "g");
 
 export class Router {
   currentRoute: Route | null = null;
@@ -87,6 +89,7 @@ export class Router {
         this.validateDestination(partialRoute.redirect);
       }
       partialRoute.params = partialRoute.path ? findParams(partialRoute.path) : [];
+      partialRoute.extractionRegExp = makeExtractionRegExp(partialRoute.path);
       this.routes[partialRoute.name] = partialRoute as Route;
       this.routeIds.push(partialRoute.name);
     }
@@ -170,19 +173,14 @@ export class Router {
   }
 
   private routeToPath(route: Route, params: RouteParams): string {
-    const path = route.path;
-    const parts = path.split("/");
-    const l = parts.length;
-    for (let i = 0; i < l; i++) {
-      const part = parts[i];
-      const match = part.match(paramRegexp);
-      if (match) {
-        const key = match[1].split(".")[0];
-        parts[i] = <string>params[key];
-      }
-    }
     const prefix = this.mode === "hash" ? "#" : "";
-    return prefix + parts.join("/");
+    return (
+      prefix +
+      route.path.replace(globalParamRegexp, (match, param) => {
+        const [key] = param.split(".");
+        return <string>params[key];
+      })
+    );
   }
 
   private currentPath(): string {
@@ -244,40 +242,47 @@ export class Router {
     if (path.startsWith("#")) {
       path = path.slice(1);
     }
-    const descrParts = route.path.split("/");
-    const targetParts = path.split("/");
-    const l = descrParts.length;
-    if (l !== targetParts.length) {
+    const paramsMatch = path.match(route.extractionRegExp);
+    if (!paramsMatch) {
       return false;
     }
     const result = {};
-    for (let i = 0; i < l; i++) {
-      const descr = descrParts[i];
-      let target: string | number = targetParts[i];
-      const match = descr.match(paramRegexp);
-      if (match) {
-        const [key, suffix] = match[1].split(".");
-        if (suffix === "number") {
-          target = parseInt(target, 10);
-        }
-        result[key] = target;
-      } else if (descr !== target) {
-        return false;
+    route.params.forEach((param, index) => {
+      const [key, suffix] = param.split(".");
+      const paramValue = paramsMatch[index + 1];
+      if (suffix === "number") {
+        return (result[key] = parseInt(paramValue, 10));
       }
-    }
+      return (result[key] = paramValue);
+    });
     return result;
   }
 }
 
 function findParams(str: string): string[] {
-  const globalParamRegexp = /\{\{(.*?)\}\}/g;
   const result: string[] = [];
   let m;
   do {
     m = globalParamRegexp.exec(str);
     if (m) {
-      result.push(m[1].split(".")[0]);
+      result.push(m[1]);
     }
   } while (m);
   return result;
 }
+
+function escapeRegExp(str: string) {
+  return str.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&");
+}
+
+function makeExtractionRegExp(path: string) {
+  // replace param strings with capture groups so that we can build a regex to match over the path
+  const extractionString = path
+    .split(paramRegexp)
+    .map((part, index) => {
+      return index % 2 ? "(.*)" : escapeRegExp(part);
+    })
+    .join("");
+  // Example: /home/{{param1}}/{{param2}} => ^\/home\/(.*)\/(.*)$
+  return new RegExp(`^${extractionString}$`);
+}

tests/router/route_component.test.ts

@@ -103,4 +103,28 @@ describe("RouteComponent", () => {
     await app.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><span>Book 1984|124</span></div>");
   });
+
+  test("can render parameterized route where params are not separated by slashes", async () => {
+    env.qweb.addTemplates(`
+        <templates>
+            <div t-name="App">
+                <RouteComponent />
+            </div>
+            <span t-name="Book">Book <t t-esc="props.title"/>|<t t-esc="props.val"/></span>
+        </templates>
+    `);
+    class Book extends Component {}
+    class App extends Component {
+      static components = { RouteComponent };
+    }
+
+    const routes = [
+      { name: "book", path: "/#title={{title}}&val={{val.number}}", component: Book },
+    ];
+    router = new TestRouter(env, routes, { mode: "hash" });
+    await router.navigate({ to: "book", params: { title: "1984", val: "123" } });
+    const app = new App();
+    await app.mount(fixture);
+    expect(fixture.innerHTML).toBe("<div><span>Book 1984|123</span></div>");
+  });
 });

tests/router/router.test.ts

@@ -1,6 +1,6 @@
 import { Destination, RouterEnv, Route } from "../../src/router/router";
 import { makeTestEnv, nextTick } from "../helpers";
-import { TestRouter } from "./test_router";
+import { TestRouter, getRouteParams } from "./test_router";
 
 let env: RouterEnv;
 let router: TestRouter | null = null;
@@ -107,60 +107,64 @@ describe("destToPath", () => {
 
 describe("getRouteParams", () => {
   test("properly match simple routes", () => {
-    router = new TestRouter(env, []);
     // simple route
-    expect(router["getRouteParams"]({ path: "/home" } as Route, "/home")).toEqual({});
+    expect(getRouteParams({ path: "/home" }, "/home")).toEqual({});
 
     // no match
-    expect(router["getRouteParams"]({ path: "/home" } as Route, "/otherpath")).toEqual(false);
+    expect(getRouteParams({ path: "/home" }, "/otherpath")).toEqual(false);
 
     // fallback route
-    expect(router["getRouteParams"]({ path: "*" } as Route, "somepath")).toEqual({});
+    expect(getRouteParams({ path: "*" }, "somepath")).toEqual({});
   });
 
   test("properly match simple routes, mode hash", () => {
-    router = new TestRouter(env, [], { mode: "hash" });
     // simple route
-    expect(router["getRouteParams"]({ path: "/home" } as Route, "#/home")).toEqual({});
+    expect(getRouteParams({ path: "/home" }, "#/home")).toEqual({});
 
     // no match
-    expect(router["getRouteParams"]({ path: "/home" } as Route, "#/otherpath")).toEqual(false);
+    expect(getRouteParams({ path: "/home" }, "#/otherpath")).toEqual(false);
 
     // fallback route
-    expect(router["getRouteParams"]({ path: "*" } as Route, "#/somepath")).toEqual({});
+    expect(getRouteParams({ path: "*" }, "#/somepath")).toEqual({});
   });
 
   test("match some parameterized routes", () => {
-    router = new TestRouter(env, []);
-    expect(router["getRouteParams"]({ path: "/invoices/{{id}}" } as Route, "/invoices/3")).toEqual({
+    expect(getRouteParams({ path: "/invoices/{{id}}" }, "/invoices/3")).toEqual({
       id: "3",
     });
   });
 
   test("match some parameterized routes, mode hash", () => {
-    router = new TestRouter(env, [], { mode: "hash" });
-    expect(router["getRouteParams"]({ path: "/invoices/{{id}}" } as Route, "#/invoices/3")).toEqual(
-      {
-        id: "3",
-      }
-    );
+    expect(getRouteParams({ path: "/invoices/{{id}}" }, "#/invoices/3")).toEqual({
+      id: "3",
+    });
   });
 
   test("can convert to number if needed", () => {
-    router = new TestRouter(env, []);
-    expect(
-      router["getRouteParams"]({ path: "/invoices/{{id.number}}" } as Route, "/invoices/3")
-    ).toEqual({
+    expect(getRouteParams({ path: "/invoices/{{id.number}}" }, "/invoices/3")).toEqual({
       id: 3,
     });
   });
 
   test("can convert to number if needed, mode: hash", () => {
-    router = new TestRouter(env, [], { mode: "hash" });
+    expect(getRouteParams({ path: "/invoices/{{id.number}}" }, "#/invoices/3")).toEqual({
+      id: 3,
+    });
+  });
+
+  test("can extract params not separated by slashes", () => {
+    expect(getRouteParams({ path: "/books/{{id.number}}-{{name}}" }, "/books/3-1984")).toEqual({
+      id: 3,
+      name: "1984",
+    });
+  });
+
+  test("can extract params not separated by slashes, mode: hash", () => {
     expect(
-      router["getRouteParams"]({ path: "/invoices/{{id.number}}" } as Route, "#/invoices/3")
+      getRouteParams({ path: "books&id={{id.number}}&name={{name}}" }, "#books&id=3&name=1984")
     ).toEqual({
       id: 3,
+      name: "1984",
     });
   });
 });

tests/router/test_router.ts

@@ -1,4 +1,5 @@
-import { Router } from "../../src/router/router";
+import { Router, Route, RouterEnv } from "../../src/router/router";
+import { makeTestEnv } from "../helpers";
 import { QWeb } from "../../src/qweb/index";
 
 export class TestRouter extends Router {
@@ -13,3 +14,13 @@ export class TestRouter extends Router {
     }
   }
 }
+
+export function getRouteParams(route: Partial<Route>, path: string) {
+  const env = <RouterEnv>makeTestEnv();
+  const router = new TestRouter(env, [route]);
+  const {
+    routeIds: [routeId],
+    routes,
+  } = router;
+  return router["getRouteParams"](routes[routeId], path);
+}