[IMP] router: add validation for destinations

src/router/Router.ts

@@ -19,7 +19,7 @@ export interface RouterEnv extends Env {
 
 export interface Destination {
   path?: string;
-  name?: string;
+  to?: string;
   params?: RouteParams;
 }
 
@@ -53,6 +53,9 @@ export class Router {
       if (partialRoute.component) {
         QWeb.register("__component__" + partialRoute.name, partialRoute.component);
       }
+      if (partialRoute.redirect) {
+          this.validateDestination(partialRoute.redirect);
+      }
       partialRoute.params = partialRoute.path ? findParams(partialRoute.path) : [];
       this.routes[partialRoute.name] = partialRoute as Route;
       this.routeIds.push(partialRoute.name);
@@ -75,16 +78,20 @@ export class Router {
   }
 
   destToUrl(dest: Destination): string {
-    if ((!dest.path && !dest.name) || (dest.path && dest.name)) {
+    this.validateDestination(dest);
-      throw new Error(`Invalid destination: ${JSON.stringify(dest)}`);
+    return dest.path || this.routeToURL(this.routes[dest.to!].path, dest.params!);
-    }
-    return dest.path || this.routeToURL(this.routes[dest.name!].path, dest.params!);
   }
 
   get currentRouteName(): string | null {
     return this.currentRoute && this.currentRoute.name;
   }
 
+  private validateDestination(dest: Destination) {
+    if ((!dest.path && !dest.to) || (dest.path && dest.to)) {
+      throw new Error(`Invalid destination: ${JSON.stringify(dest)}`);
+    }
+  }
+
   private routeToURL(path: string, params: RouteParams): string {
     const parts = path.split("/");
     const l = parts.length;

tests/router/directive.test.ts

@@ -43,12 +43,12 @@ describe("router directive t-routecomponent", () => {
     ];
 
     router = new TestRouter(env, routes, { mode: "history" });
-    router.navigate({ name: "about" });
+    router.navigate({ to: "about" });
     const app = new App(env);
     await app.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><span>About</span></div>");
 
-    router.navigate({ name: "users" });
+    router.navigate({ to: "users" });
     await nextTick();
     expect(fixture.innerHTML).toBe("<div><span>Users</span></div>");
     expect(env.qweb.templates.App.fn.toString()).toMatchSnapshot();
@@ -70,7 +70,7 @@ describe("router directive t-routecomponent", () => {
 
     const routes = [{ name: "book", path: "/book/{{title}}", component: Book }];
     router = new TestRouter(env, routes, { mode: "history" });
-    router.navigate({ name: "book", params: { title: "1984" } });
+    router.navigate({ to: "book", params: { title: "1984" } });
     const app = new App(env);
     await app.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><span>Book 1984</span></div>");
@@ -97,7 +97,7 @@ describe("router directive t-routecomponent", () => {
 
     const routes = [{ name: "book", path: "/book/{{title}}/{{val.number}}", component: Book }];
     router = new TestRouter(env, routes, { mode: "history" });
-    router.navigate({ name: "book", params: { title: "1984", val: "123" } });
+    router.navigate({ to: "book", params: { title: "1984", val: "123" } });
     const app = new App(env);
     await app.mount(fixture);
     expect(fixture.innerHTML).toBe("<div><span>Book 1984|124</span></div>");

tests/router/router.test.ts

@@ -16,6 +16,16 @@ afterEach(() => {
   router = null;
 });
 
+describe("router miscellaneous", () => {
+  test("validate routes shape", () => {
+    expect(() => {
+      router = new TestRouter(env, [
+        { name: "someroute", path: "/some/path", redirect: { abc: "hey" } as Destination }
+      ]);
+    }).toThrow(`Invalid destination: {"abc":"hey"}`);
+  });
+});
+
 describe("routeToURL", () => {
   const routeToURL = Router.prototype["routeToURL"];
   test("simple non parameterized path", () => {
@@ -37,11 +47,11 @@ describe("destToURL", () => {
     }).toThrow('Invalid destination: {"abc":123}');
 
     expect(() => {
-      router!.destToUrl({ name: "someroute" } as Destination);
+      router!.destToUrl({ to: "someroute" } as Destination);
     }).not.toThrow();
 
     expect(() => {
-      router!.destToUrl({ path: "/someroute", name: "otherroute" } as Destination);
+      router!.destToUrl({ path: "/someroute", to: "otherroute" } as Destination);
     }).toThrow();
   });
 });
@@ -76,10 +86,10 @@ describe("redirect", () => {
   test("can redirect to other route", () => {
     router = new TestRouter(env, [
       { name: "routea", path: "/some/path" },
-      { name: "routeb", path: "/some/other/path", redirect: { name: "routea" } }
+      { name: "routeb", path: "/some/other/path", redirect: { to: "routea" } }
     ]);
 
-    router.navigate({ name: "routeb" });
+    router.navigate({ to: "routeb" });
     expect(window.location.pathname).toBe("/some/path");
     expect(router.currentRouteName).toBe("routea");
   });
@@ -90,7 +100,7 @@ describe("redirect", () => {
       { name: "routeb", path: "/some/other/path", redirect: { path: "/some/path" } }
     ]);
 
-    router.navigate({ name: "routeb" });
+    router.navigate({ to: "routeb" });
     expect(window.location.pathname).toBe("/some/path");
     expect(router.currentRouteName).toBe("routea");
   });