From e52a6daae83e1c7798d27891dcbd03d4d306b031 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 19 Sep 2020 13:17:08 +0200
Subject: [PATCH 1/9] core-api/routing: initial RouteRefRegistry implementation

---
 .../src/routing/RouteRefRegistry.test.ts      |  67 ++++++++++++
 .../core-api/src/routing/RouteRefRegistry.ts  | 103 ++++++++++++++++++
 2 files changed, 170 insertions(+)
 create mode 100644 packages/core-api/src/routing/RouteRefRegistry.test.ts
 create mode 100644 packages/core-api/src/routing/RouteRefRegistry.ts

diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
new file mode 100644
index 0000000000..addf7dec80
--- /dev/null
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -0,0 +1,67 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { RouteRefRegistry } from './RouteRefRegistry';
+
+const ref1 = {};
+const ref11 = {};
+const ref12 = {};
+const ref121 = {};
+const ref2 = {};
+
+describe('RouteRefRegistry', () => {
+  it('should be constructed with a root route', () => {
+    const registry = new RouteRefRegistry();
+    expect(registry.resolveRoute([], [])).toBe('');
+  });
+
+  it('should register and resolve some routes', () => {
+    const registry = new RouteRefRegistry();
+    expect(registry.registerRoute([ref1], '1')).toBe(true);
+    expect(registry.registerRoute([ref1, ref11], '11')).toBe(true);
+    expect(registry.registerRoute([ref1, ref12], '12')).toBe(true);
+    expect(registry.registerRoute([ref1, ref12, ref121], '121')).toBe(true);
+    expect(registry.registerRoute([ref1, ref12, ref121], 'duplicate')).toBe(
+      false,
+    );
+    expect(registry.registerRoute([ref1, ref12], 'duplicate')).toBe(false);
+    expect(registry.registerRoute([ref2], '2')).toBe(true);
+    expect(registry.registerRoute([ref2], 'duplicate')).toBe(false);
+
+    expect(registry.resolveRoute([], [ref1])).toBe('1');
+    expect(registry.resolveRoute([], [ref11])).toBe(undefined);
+    expect(registry.resolveRoute([], [ref1, ref11])).toBe('11');
+    expect(registry.resolveRoute([ref1], [ref11])).toBe('11');
+    expect(registry.resolveRoute([ref1], [ref2])).toBe('2');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [])).toBe('121');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref121])).toBe('121');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref12, ref121])).toBe(
+      '121',
+    );
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref12])).toBe('12');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref1])).toBe('1');
+  });
+
+  it('should throw when registering routes incorrectly', () => {
+    const registry = new RouteRefRegistry();
+    expect(() => {
+      registry.registerRoute([ref1, ref11], '11');
+    }).toThrow('Could not find parent for new routing node');
+    expect(() => {
+      registry.registerRoute([], '11');
+    }).toThrow('Must provide at least 1 route to add routing node');
+  });
+});
diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts
new file mode 100644
index 0000000000..3b6cb9e768
--- /dev/null
+++ b/packages/core-api/src/routing/RouteRefRegistry.ts
@@ -0,0 +1,103 @@
+/*
+ * Copyright 2020 Spotify AB
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export type RouteRefResolver = {
+  resolveRoute(from: unknown[], to: unknown[]): string;
+};
+
+class Node {
+  readonly children = new Map<unknown, Node>();
+
+  constructor(readonly path: string, readonly parent: Node | undefined) {}
+
+  /**
+   * Look up a node in the tree given a path.
+   */
+  findNode(routes: unknown[]): Node | undefined {
+    // eslint-disable-next-line consistent-this
+    let node: Node | undefined = this;
+
+    for (let i = 0; i < routes.length; i++) {
+      node = node?.children.get(routes[i]);
+    }
+
+    return node;
+  }
+
+  /**
+   * Assigns a path to a leaf node in the routing tree. All ancestor
+   * nodes of the new leaf node must already exist, or an error will be thrown.
+   *
+   * Returns true if the node was added, or false if the node already existed.
+   */
+  addNode(routes: unknown[], path: string): boolean {
+    if (routes.length === 0) {
+      throw new Error('Must provide at least 1 route to add routing node');
+    }
+
+    const parentNode = this.findNode(routes.slice(0, -1));
+    if (!parentNode) {
+      throw new Error('Could not find parent for new routing node');
+    }
+
+    const lastRoute = routes[routes.length - 1];
+    if (parentNode.children.has(lastRoute)) {
+      return false;
+    }
+
+    parentNode.children.set(lastRoute, new Node(path, parentNode));
+    return true;
+  }
+}
+
+/**
+ * A registry for resolving route refs into concrete string routes.
+ */
+export class RouteRefRegistry {
+  private readonly root = new Node('', undefined);
+
+  /**
+   * Register a new leaf path for a sequence of routes. All ancestor
+   * routes must already exist.
+   */
+  registerRoute(routes: unknown[], path: string): boolean {
+    return this.root.addNode(routes, path);
+  }
+
+  /**
+   * Resolve a route from a point in the routing tree.
+   *
+   * The route referenced by `from` must exist, and is the starting
+   * point for the search, walking up the tree until a subtree that
+   * matches the routes reference in `to` are found.
+   *
+   * If `from` is empty, the search starts and ends at the root node.
+   * If `to` is empty, the route referenced by `from` will always be returned.
+   */
+  resolveRoute(from: unknown[], to: unknown[]): string | undefined {
+    let fromNode = this.root.findNode(from);
+
+    while (fromNode) {
+      const resolvedNode = fromNode.findNode(to);
+      if (resolvedNode) {
+        return resolvedNode.path;
+      }
+      fromNode = fromNode.parent;
+    }
+
+    return undefined;
+  }
+}

From 558ae5593ecf1f2d823b37df1da25342cd40062a Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 19 Sep 2020 14:56:27 +0200
Subject: [PATCH 2/9] core-api/routing: implement parameterized and absolute
 path resolution

---
 .../src/routing/RouteRefRegistry.test.ts      | 34 +++++-----
 .../core-api/src/routing/RouteRefRegistry.ts  | 67 ++++++++++++++++---
 2 files changed, 74 insertions(+), 27 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
index addf7dec80..7547de1eb4 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.test.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -14,13 +14,13 @@
  * limitations under the License.
  */
 
-import { RouteRefRegistry } from './RouteRefRegistry';
+import { RouteRefRegistry, resolveRoute } from './RouteRefRegistry';
 
-const ref1 = {};
-const ref11 = {};
-const ref12 = {};
-const ref121 = {};
-const ref2 = {};
+const ref1 = { [resolveRoute]: (path: string) => path };
+const ref11 = { [resolveRoute]: (path: string) => path };
+const ref12 = { [resolveRoute]: (path: string) => path };
+const ref121 = { [resolveRoute]: (path: string) => path };
+const ref2 = { [resolveRoute]: (path: string) => path };
 
 describe('RouteRefRegistry', () => {
   it('should be constructed with a root route', () => {
@@ -41,18 +41,20 @@ describe('RouteRefRegistry', () => {
     expect(registry.registerRoute([ref2], '2')).toBe(true);
     expect(registry.registerRoute([ref2], 'duplicate')).toBe(false);
 
-    expect(registry.resolveRoute([], [ref1])).toBe('1');
+    expect(registry.resolveRoute([], [ref1])).toBe('/1');
     expect(registry.resolveRoute([], [ref11])).toBe(undefined);
-    expect(registry.resolveRoute([], [ref1, ref11])).toBe('11');
-    expect(registry.resolveRoute([ref1], [ref11])).toBe('11');
-    expect(registry.resolveRoute([ref1], [ref2])).toBe('2');
-    expect(registry.resolveRoute([ref1, ref12, ref121], [])).toBe('121');
-    expect(registry.resolveRoute([ref1, ref12, ref121], [ref121])).toBe('121');
-    expect(registry.resolveRoute([ref1, ref12, ref121], [ref12, ref121])).toBe(
-      '121',
+    expect(registry.resolveRoute([], [ref1, ref11])).toBe('/1/11');
+    expect(registry.resolveRoute([ref1], [ref11])).toBe('/1/11');
+    expect(registry.resolveRoute([ref1], [ref2])).toBe('/2');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [])).toBe('/1/12/121');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref121])).toBe(
+      '/1/12/121',
     );
-    expect(registry.resolveRoute([ref1, ref12, ref121], [ref12])).toBe('12');
-    expect(registry.resolveRoute([ref1, ref12, ref121], [ref1])).toBe('1');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref12, ref121])).toBe(
+      '/1/12/121',
+    );
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref12])).toBe('/1/12');
+    expect(registry.resolveRoute([ref1, ref12, ref121], [ref1])).toBe('/1');
   });
 
   it('should throw when registering routes incorrectly', () => {
diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts
index 3b6cb9e768..7d6260f78a 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.ts
@@ -14,21 +14,30 @@
  * limitations under the License.
  */
 
+export const resolveRoute = Symbol('resolve-route');
+
+type ConcreteRoute = {
+  [resolveRoute](path: string): string;
+};
+
+const rootRoute: ConcreteRoute = {
+  [resolveRoute]: () => '',
+};
+
 export type RouteRefResolver = {
-  resolveRoute(from: unknown[], to: unknown[]): string;
+  resolveRoute(from: ConcreteRoute[], to: ConcreteRoute[]): string;
 };
 
 class Node {
-  readonly children = new Map<unknown, Node>();
+  readonly children = new Map<ConcreteRoute, Node>();
 
   constructor(readonly path: string, readonly parent: Node | undefined) {}
 
   /**
    * Look up a node in the tree given a path.
    */
-  findNode(routes: unknown[]): Node | undefined {
-    // eslint-disable-next-line consistent-this
-    let node: Node | undefined = this;
+  findNode(routes: ConcreteRoute[]): Node | undefined {
+    let node = this as Node | undefined;
 
     for (let i = 0; i < routes.length; i++) {
       node = node?.children.get(routes[i]);
@@ -43,7 +52,7 @@ class Node {
    *
    * Returns true if the node was added, or false if the node already existed.
    */
-  addNode(routes: unknown[], path: string): boolean {
+  addNode(routes: ConcreteRoute[], path: string): boolean {
     if (routes.length === 0) {
       throw new Error('Must provide at least 1 route to add routing node');
     }
@@ -61,6 +70,35 @@ class Node {
     parentNode.children.set(lastRoute, new Node(path, parentNode));
     return true;
   }
+
+  /**
+   * Resolve an absolute URL that represents this node in the routing tree, using
+   * using the supplied concrete routes and ancestors of this node.
+   *
+   * The length of the provided routes array must match the depth of
+   * the routing tree that this node is at, or an error will be thrown.
+   */
+  resolve(routes: ConcreteRoute[]) {
+    const parts = Array(routes.length);
+
+    let node = this as Node | undefined;
+    for (let i = routes.length - 1; i >= 0; i--) {
+      if (!node) {
+        throw new Error('Route resolve missing required parent');
+      }
+
+      const route = routes[i];
+      parts[i] = route[resolveRoute](node.path);
+
+      node = node.parent;
+    }
+
+    if (node) {
+      throw new Error('Route resolve did not reach root');
+    }
+
+    return parts.join('/');
+  }
 }
 
 /**
@@ -73,12 +111,12 @@ export class RouteRefRegistry {
    * Register a new leaf path for a sequence of routes. All ancestor
    * routes must already exist.
    */
-  registerRoute(routes: unknown[], path: string): boolean {
+  registerRoute(routes: ConcreteRoute[], path: string): boolean {
     return this.root.addNode(routes, path);
   }
 
   /**
-   * Resolve a route from a point in the routing tree.
+   * Resolve an absolute path from a point in the routing tree.
    *
    * The route referenced by `from` must exist, and is the starting
    * point for the search, walking up the tree until a subtree that
@@ -87,14 +125,21 @@ export class RouteRefRegistry {
    * If `from` is empty, the search starts and ends at the root node.
    * If `to` is empty, the route referenced by `from` will always be returned.
    */
-  resolveRoute(from: unknown[], to: unknown[]): string | undefined {
-    let fromNode = this.root.findNode(from);
+  resolveRoute(from: ConcreteRoute[], to: ConcreteRoute[]): string | undefined {
+    // Keep track of the `from` routes and pop the last ones as we traverse up
+    // the routing tree. The list of concrete routes that we're passing to
+    // `node.resolve()` should only include the ones in the resolve path.
+    const concreteStack = from.slice();
 
+    let fromNode = this.root.findNode(from);
     while (fromNode) {
       const resolvedNode = fromNode.findNode(to);
       if (resolvedNode) {
-        return resolvedNode.path;
+        return resolvedNode.resolve([rootRoute].concat(concreteStack, to));
       }
+
+      // Search at this level of the tree failed, move up to parent
+      concreteStack.pop();
       fromNode = fromNode.parent;
     }
 

From 988ebe7139d332b648a5a7d77d9b71be07373b26 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 19 Sep 2020 15:16:24 +0200
Subject: [PATCH 3/9] core-api/routing: make RouteRefs immutable + move out
 some types

---
 packages/core-api/src/routing/RouteRef.ts     | 21 +++++++------------
 .../src/routing/RouteRefRegistry.test.ts      |  3 ++-
 .../core-api/src/routing/RouteRefRegistry.ts  |  6 +-----
 packages/core-api/src/routing/index.ts        |  3 +--
 packages/core-api/src/routing/types.ts        | 13 ++++++------
 5 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts
index 9f9980ed0c..8a03680791 100644
--- a/packages/core-api/src/routing/RouteRef.ts
+++ b/packages/core-api/src/routing/RouteRef.ts
@@ -14,30 +14,25 @@
  * limitations under the License.
  */
 
-import type { RouteRefConfig, RouteRefOverrideConfig } from './types';
-
-export class MutableRouteRef {
-  private effectiveConfig: RouteRefConfig = this.config;
+import type { RouteRefConfig } from './types';
 
+export class AbsoluteRouteRef {
   constructor(private readonly config: RouteRefConfig) {}
 
-  override(overrideConfig: RouteRefOverrideConfig) {
-    this.effectiveConfig = { ...this.config, ...overrideConfig };
-  }
-
   get icon() {
-    return this.effectiveConfig.icon;
+    return this.config.icon;
   }
 
+  // TODO(Rugvip): Remove this, routes are looked up via the registry instead
   get path() {
-    return this.effectiveConfig.path;
+    return this.config.path;
   }
 
   get title() {
-    return this.effectiveConfig.title;
+    return this.config.title;
   }
 }
 
-export function createRouteRef(config: RouteRefConfig): MutableRouteRef {
-  return new MutableRouteRef(config);
+export function createRouteRef(config: RouteRefConfig): AbsoluteRouteRef {
+  return new AbsoluteRouteRef(config);
 }
diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
index 7547de1eb4..76272cb9f7 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.test.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import { RouteRefRegistry, resolveRoute } from './RouteRefRegistry';
+import { RouteRefRegistry } from './RouteRefRegistry';
+import { resolveRoute } from './types';
 
 const ref1 = { [resolveRoute]: (path: string) => path };
 const ref11 = { [resolveRoute]: (path: string) => path };
diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts
index 7d6260f78a..3b9bf4a7a9 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.ts
@@ -14,11 +14,7 @@
  * limitations under the License.
  */
 
-export const resolveRoute = Symbol('resolve-route');
-
-type ConcreteRoute = {
-  [resolveRoute](path: string): string;
-};
+import { ConcreteRoute, resolveRoute } from './types';
 
 const rootRoute: ConcreteRoute = {
   [resolveRoute]: () => '',
diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts
index 67d4c82167..1b80c0838e 100644
--- a/packages/core-api/src/routing/index.ts
+++ b/packages/core-api/src/routing/index.ts
@@ -14,6 +14,5 @@
  * limitations under the License.
  */
 
-export * from './types';
+export type { RouteRef, RouteRefConfig, ConcreteRoute } from './types';
 export { createRouteRef } from './RouteRef';
-export type { MutableRouteRef } from './RouteRef';
diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts
index 515dc31de6..9e5e2bb3a7 100644
--- a/packages/core-api/src/routing/types.ts
+++ b/packages/core-api/src/routing/types.ts
@@ -16,7 +16,14 @@
 
 import { IconComponent } from '../icons';
 
+export const resolveRoute = Symbol('resolve-route');
+
+export type ConcreteRoute = {
+  [resolveRoute](path: string): string;
+};
+
 export type RouteRef = {
+  // TODO(Rugvip): Remove path, look up via registry instead
   path: string;
   icon?: IconComponent;
   title: string;
@@ -27,9 +34,3 @@ export type RouteRefConfig = {
   icon?: IconComponent;
   title: string;
 };
-
-export type RouteRefOverrideConfig = {
-  path?: string;
-  icon?: IconComponent;
-  title?: string;
-};

From 9155138168be2014c2b17ed831f00c6345b65718 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 19 Sep 2020 15:21:43 +0200
Subject: [PATCH 4/9] core-api/routing: make RouteRef implement ConcreteRoute
 and use in registry test

---
 packages/core-api/src/routing/RouteRef.ts           |  8 ++++++--
 .../core-api/src/routing/RouteRefRegistry.test.ts   | 13 +++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts
index 8a03680791..cc36446f8b 100644
--- a/packages/core-api/src/routing/RouteRef.ts
+++ b/packages/core-api/src/routing/RouteRef.ts
@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import type { RouteRefConfig } from './types';
+import { ConcreteRoute, resolveRoute, RouteRefConfig } from './types';
 
-export class AbsoluteRouteRef {
+export class AbsoluteRouteRef implements ConcreteRoute {
   constructor(private readonly config: RouteRefConfig) {}
 
   get icon() {
@@ -31,6 +31,10 @@ export class AbsoluteRouteRef {
   get title() {
     return this.config.title;
   }
+
+  [resolveRoute](path: string) {
+    return path;
+  }
 }
 
 export function createRouteRef(config: RouteRefConfig): AbsoluteRouteRef {
diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
index 76272cb9f7..91bbc70117 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.test.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -15,13 +15,14 @@
  */
 
 import { RouteRefRegistry } from './RouteRefRegistry';
-import { resolveRoute } from './types';
+import { createRouteRef } from './RouteRef';
 
-const ref1 = { [resolveRoute]: (path: string) => path };
-const ref11 = { [resolveRoute]: (path: string) => path };
-const ref12 = { [resolveRoute]: (path: string) => path };
-const ref121 = { [resolveRoute]: (path: string) => path };
-const ref2 = { [resolveRoute]: (path: string) => path };
+const dummyConfig = { path: '/', icon: null, title: 'my-title' };
+const ref1 = createRouteRef(dummyConfig);
+const ref11 = createRouteRef(dummyConfig);
+const ref12 = createRouteRef(dummyConfig);
+const ref121 = createRouteRef(dummyConfig);
+const ref2 = createRouteRef(dummyConfig);
 
 describe('RouteRefRegistry', () => {
   it('should be constructed with a root route', () => {

From 0f3e9b29d44e1add0693a79abdcdbed11051be96 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sat, 19 Sep 2020 16:32:41 +0200
Subject: [PATCH 5/9] core-api/routing: initial subroute implementation

---
 packages/core-api/src/routing/RouteRef.ts     | 41 ++++++++++++++++++-
 .../src/routing/RouteRefRegistry.test.ts      | 39 +++++++++++++++++-
 .../core-api/src/routing/RouteRefRegistry.ts  | 20 +++++----
 packages/core-api/src/routing/types.ts        |  7 +++-
 4 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts
index cc36446f8b..51bb9ee1a4 100644
--- a/packages/core-api/src/routing/RouteRef.ts
+++ b/packages/core-api/src/routing/RouteRef.ts
@@ -14,7 +14,36 @@
  * limitations under the License.
  */
 
-import { ConcreteRoute, resolveRoute, RouteRefConfig } from './types';
+import { ConcreteRoute, ref, resolveRoute, RouteRefConfig } from './types';
+import { generatePath } from 'react-router-dom';
+
+type SubRouteConfig = {
+  path: string;
+};
+
+export class SubRouteRef<T extends { [name in string]: string }> {
+  constructor(
+    private readonly parent: ConcreteRoute,
+    private readonly config: SubRouteConfig,
+  ) {}
+
+  [ref]() {
+    return this;
+  }
+
+  link(params: T): ConcreteRoute {
+    const selfRef = this as unknown;
+
+    return {
+      [ref]: () => selfRef,
+      [resolveRoute]: (path: string) => {
+        const ownPart = generatePath(this.config.path, params);
+        const parentPart = this.parent[resolveRoute](path);
+        return parentPart + ownPart;
+      },
+    };
+  }
+}
 
 export class AbsoluteRouteRef implements ConcreteRoute {
   constructor(private readonly config: RouteRefConfig) {}
@@ -32,6 +61,16 @@ export class AbsoluteRouteRef implements ConcreteRoute {
     return this.config.title;
   }
 
+  createSubRoute<T extends { [name in string]: string }>(
+    config: SubRouteConfig,
+  ) {
+    return new SubRouteRef<T>(this, config);
+  }
+
+  [ref]() {
+    return this;
+  }
+
   [resolveRoute](path: string) {
     return path;
   }
diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
index 91bbc70117..2849bcf32b 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.test.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -17,12 +17,14 @@
 import { RouteRefRegistry } from './RouteRefRegistry';
 import { createRouteRef } from './RouteRef';
 
-const dummyConfig = { path: '/', icon: null, title: 'my-title' };
+const dummyConfig = { path: '/', icon: () => null, title: 'my-title' };
 const ref1 = createRouteRef(dummyConfig);
 const ref11 = createRouteRef(dummyConfig);
 const ref12 = createRouteRef(dummyConfig);
 const ref121 = createRouteRef(dummyConfig);
 const ref2 = createRouteRef(dummyConfig);
+const ref2a = ref2.createSubRoute({ path: '/a' });
+const ref2b = ref2.createSubRoute<{ id: string }>({ path: '/b/:id' });
 
 describe('RouteRefRegistry', () => {
   it('should be constructed with a root route', () => {
@@ -30,7 +32,7 @@ describe('RouteRefRegistry', () => {
     expect(registry.resolveRoute([], [])).toBe('');
   });
 
-  it('should register and resolve some routes', () => {
+  it('should register and resolve some absolute routes', () => {
     const registry = new RouteRefRegistry();
     expect(registry.registerRoute([ref1], '1')).toBe(true);
     expect(registry.registerRoute([ref1, ref11], '11')).toBe(true);
@@ -59,6 +61,39 @@ describe('RouteRefRegistry', () => {
     expect(registry.resolveRoute([ref1, ref12, ref121], [ref1])).toBe('/1');
   });
 
+  it('should register and resolve with sub routes', () => {
+    const registry = new RouteRefRegistry();
+    expect(registry.registerRoute([ref1], '1')).toBe(true);
+    expect(registry.registerRoute([ref2], '2')).toBe(true);
+    expect(registry.registerRoute([ref2a], '2')).toBe(true);
+    expect(registry.registerRoute([ref2a, ref1], '1')).toBe(true);
+    expect(registry.registerRoute([ref2a, ref2], '2')).toBe(true);
+    expect(registry.registerRoute([ref2b], '2')).toBe(true);
+    expect(registry.registerRoute([ref2b, ref1], '1')).toBe(true);
+    expect(registry.registerRoute([ref2b, ref2], '2')).toBe(true);
+
+    expect(registry.resolveRoute([], [ref1])).toBe('/1');
+    expect(registry.resolveRoute([], [ref2])).toBe('/2');
+    expect(registry.resolveRoute([], [ref2a.link({}), ref1])).toBe('/2/a/1');
+    expect(registry.resolveRoute([], [ref2a.link({}), ref2])).toBe('/2/a/2');
+    expect(registry.resolveRoute([ref2a.link({})], [ref2])).toBe('/2/a/2');
+    expect(registry.resolveRoute([ref2a.link({}), ref1], [ref2])).toBe(
+      '/2/a/2',
+    );
+    expect(registry.resolveRoute([], [ref2b.link({ id: 'abc' }), ref1])).toBe(
+      '/2/b/abc/1',
+    );
+    expect(registry.resolveRoute([], [ref2b.link({ id: 'xyz' }), ref2])).toBe(
+      '/2/b/xyz/2',
+    );
+    expect(registry.resolveRoute([ref2b.link({ id: 'abc' })], [ref2])).toBe(
+      '/2/b/abc/2',
+    );
+    expect(
+      registry.resolveRoute([ref2b.link({ id: 'abc' }), ref1], [ref2]),
+    ).toBe('/2/b/abc/2');
+  });
+
   it('should throw when registering routes incorrectly', () => {
     const registry = new RouteRefRegistry();
     expect(() => {
diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts
index 3b9bf4a7a9..7377a90672 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.ts
@@ -14,9 +14,12 @@
  * limitations under the License.
  */
 
-import { ConcreteRoute, resolveRoute } from './types';
+import { ConcreteRoute, ref, resolveRoute, ReferencedRoute } from './types';
 
 const rootRoute: ConcreteRoute = {
+  [ref]() {
+    return this;
+  },
   [resolveRoute]: () => '',
 };
 
@@ -25,18 +28,18 @@ export type RouteRefResolver = {
 };
 
 class Node {
-  readonly children = new Map<ConcreteRoute, Node>();
+  readonly children = new Map<unknown, Node>();
 
   constructor(readonly path: string, readonly parent: Node | undefined) {}
 
   /**
    * Look up a node in the tree given a path.
    */
-  findNode(routes: ConcreteRoute[]): Node | undefined {
+  findNode(routes: ReferencedRoute[]): Node | undefined {
     let node = this as Node | undefined;
 
     for (let i = 0; i < routes.length; i++) {
-      node = node?.children.get(routes[i]);
+      node = node?.children.get(routes[i][ref]());
     }
 
     return node;
@@ -48,7 +51,7 @@ class Node {
    *
    * Returns true if the node was added, or false if the node already existed.
    */
-  addNode(routes: ConcreteRoute[], path: string): boolean {
+  addNode(routes: ReferencedRoute[], path: string): boolean {
     if (routes.length === 0) {
       throw new Error('Must provide at least 1 route to add routing node');
     }
@@ -59,11 +62,12 @@ class Node {
     }
 
     const lastRoute = routes[routes.length - 1];
-    if (parentNode.children.has(lastRoute)) {
+    const lastRouteRef = lastRoute[ref]();
+    if (parentNode.children.has(lastRouteRef)) {
       return false;
     }
 
-    parentNode.children.set(lastRoute, new Node(path, parentNode));
+    parentNode.children.set(lastRouteRef, new Node(path, parentNode));
     return true;
   }
 
@@ -107,7 +111,7 @@ export class RouteRefRegistry {
    * Register a new leaf path for a sequence of routes. All ancestor
    * routes must already exist.
    */
-  registerRoute(routes: ConcreteRoute[], path: string): boolean {
+  registerRoute(routes: ReferencedRoute[], path: string): boolean {
     return this.root.addNode(routes, path);
   }
 
diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts
index 9e5e2bb3a7..291029c3f2 100644
--- a/packages/core-api/src/routing/types.ts
+++ b/packages/core-api/src/routing/types.ts
@@ -17,8 +17,13 @@
 import { IconComponent } from '../icons';
 
 export const resolveRoute = Symbol('resolve-route');
+export const ref = Symbol('route-ref');
 
-export type ConcreteRoute = {
+export type ReferencedRoute = {
+  [ref](): unknown;
+};
+
+export type ConcreteRoute = ReferencedRoute & {
   [resolveRoute](path: string): string;
 };
 

From 735c1277d1c327b299c5c4c90599e734e5be64c8 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sun, 20 Sep 2020 13:21:55 +0200
Subject: [PATCH 6/9] core-api/routing: switch ref to prop and rename to
 routeReference

---
 packages/core-api/src/routing/RouteRef.ts     | 19 ++++++++++++-------
 .../core-api/src/routing/RouteRefRegistry.ts  | 13 +++++++++----
 packages/core-api/src/routing/types.ts        |  4 ++--
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts
index 51bb9ee1a4..5e7a4294d9 100644
--- a/packages/core-api/src/routing/RouteRef.ts
+++ b/packages/core-api/src/routing/RouteRef.ts
@@ -14,28 +14,33 @@
  * limitations under the License.
  */
 
-import { ConcreteRoute, ref, resolveRoute, RouteRefConfig } from './types';
+import {
+  ConcreteRoute,
+  routeReference,
+  ReferencedRoute,
+  resolveRoute,
+  RouteRefConfig,
+} from './types';
 import { generatePath } from 'react-router-dom';
 
 type SubRouteConfig = {
   path: string;
 };
 
-export class SubRouteRef<T extends { [name in string]: string }> {
+export class SubRouteRef<T extends { [name in string]: string }>
+  implements ReferencedRoute {
   constructor(
     private readonly parent: ConcreteRoute,
     private readonly config: SubRouteConfig,
   ) {}
 
-  [ref]() {
+  get [routeReference]() {
     return this;
   }
 
   link(params: T): ConcreteRoute {
-    const selfRef = this as unknown;
-
     return {
-      [ref]: () => selfRef,
+      [routeReference]: this,
       [resolveRoute]: (path: string) => {
         const ownPart = generatePath(this.config.path, params);
         const parentPart = this.parent[resolveRoute](path);
@@ -67,7 +72,7 @@ export class AbsoluteRouteRef implements ConcreteRoute {
     return new SubRouteRef<T>(this, config);
   }
 
-  [ref]() {
+  get [routeReference]() {
     return this;
   }
 
diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts
index 7377a90672..4bc824ac3f 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.ts
@@ -14,10 +14,15 @@
  * limitations under the License.
  */
 
-import { ConcreteRoute, ref, resolveRoute, ReferencedRoute } from './types';
+import {
+  ConcreteRoute,
+  routeReference,
+  resolveRoute,
+  ReferencedRoute,
+} from './types';
 
 const rootRoute: ConcreteRoute = {
-  [ref]() {
+  get [routeReference]() {
     return this;
   },
   [resolveRoute]: () => '',
@@ -39,7 +44,7 @@ class Node {
     let node = this as Node | undefined;
 
     for (let i = 0; i < routes.length; i++) {
-      node = node?.children.get(routes[i][ref]());
+      node = node?.children.get(routes[i][routeReference]);
     }
 
     return node;
@@ -62,7 +67,7 @@ class Node {
     }
 
     const lastRoute = routes[routes.length - 1];
-    const lastRouteRef = lastRoute[ref]();
+    const lastRouteRef = lastRoute[routeReference];
     if (parentNode.children.has(lastRouteRef)) {
       return false;
     }
diff --git a/packages/core-api/src/routing/types.ts b/packages/core-api/src/routing/types.ts
index 291029c3f2..162ac74bde 100644
--- a/packages/core-api/src/routing/types.ts
+++ b/packages/core-api/src/routing/types.ts
@@ -17,10 +17,10 @@
 import { IconComponent } from '../icons';
 
 export const resolveRoute = Symbol('resolve-route');
-export const ref = Symbol('route-ref');
+export const routeReference = Symbol('route-ref');
 
 export type ReferencedRoute = {
-  [ref](): unknown;
+  [routeReference]: unknown;
 };
 
 export type ConcreteRoute = ReferencedRoute & {

From 240366572ca5d4c37e9da05f5b964c66a831872c Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sun, 20 Sep 2020 14:41:42 +0200
Subject: [PATCH 7/9] core-api/routing: allow duplicate registration if paths
 match

---
 packages/core-api/src/routing/RouteRefRegistry.test.ts | 1 +
 packages/core-api/src/routing/RouteRefRegistry.ts      | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
index 2849bcf32b..91d730de40 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.test.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -44,6 +44,7 @@ describe('RouteRefRegistry', () => {
     expect(registry.registerRoute([ref1, ref12], 'duplicate')).toBe(false);
     expect(registry.registerRoute([ref2], '2')).toBe(true);
     expect(registry.registerRoute([ref2], 'duplicate')).toBe(false);
+    expect(registry.registerRoute([ref2], '2')).toBe(true);
 
     expect(registry.resolveRoute([], [ref1])).toBe('/1');
     expect(registry.resolveRoute([], [ref11])).toBe(undefined);
diff --git a/packages/core-api/src/routing/RouteRefRegistry.ts b/packages/core-api/src/routing/RouteRefRegistry.ts
index 4bc824ac3f..7e55cbe8f7 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.ts
@@ -68,8 +68,10 @@ class Node {
 
     const lastRoute = routes[routes.length - 1];
     const lastRouteRef = lastRoute[routeReference];
-    if (parentNode.children.has(lastRouteRef)) {
-      return false;
+
+    const existingNode = parentNode.children.get(lastRouteRef);
+    if (existingNode) {
+      return existingNode.path === path;
     }
 
     parentNode.children.set(lastRouteRef, new Node(path, parentNode));

From 8804c5e338b19ef27eeea37d47cf567415a62353 Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Sun, 20 Sep 2020 14:49:46 +0200
Subject: [PATCH 8/9] core-api/routing: only require args in link call if
 routeRef has params

---
 packages/core-api/src/routing/RouteRef.ts              |  8 ++++----
 packages/core-api/src/routing/RouteRefRegistry.test.ts | 10 ++++------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts
index 5e7a4294d9..c8b0855de7 100644
--- a/packages/core-api/src/routing/RouteRef.ts
+++ b/packages/core-api/src/routing/RouteRef.ts
@@ -27,7 +27,7 @@ type SubRouteConfig = {
   path: string;
 };
 
-export class SubRouteRef<T extends { [name in string]: string }>
+export class SubRouteRef<T extends { [name in string]: string } | never = never>
   implements ReferencedRoute {
   constructor(
     private readonly parent: ConcreteRoute,
@@ -38,11 +38,11 @@ export class SubRouteRef<T extends { [name in string]: string }>
     return this;
   }
 
-  link(params: T): ConcreteRoute {
+  link<Args extends T extends never ? [] : [T]>(...args: Args): ConcreteRoute {
     return {
       [routeReference]: this,
       [resolveRoute]: (path: string) => {
-        const ownPart = generatePath(this.config.path, params);
+        const ownPart = generatePath(this.config.path, args[0] ?? {});
         const parentPart = this.parent[resolveRoute](path);
         return parentPart + ownPart;
       },
@@ -66,7 +66,7 @@ export class AbsoluteRouteRef implements ConcreteRoute {
     return this.config.title;
   }
 
-  createSubRoute<T extends { [name in string]: string }>(
+  createSubRoute<T extends { [name in string]: string } | never = never>(
     config: SubRouteConfig,
   ) {
     return new SubRouteRef<T>(this, config);
diff --git a/packages/core-api/src/routing/RouteRefRegistry.test.ts b/packages/core-api/src/routing/RouteRefRegistry.test.ts
index 91d730de40..fa1ef584f1 100644
--- a/packages/core-api/src/routing/RouteRefRegistry.test.ts
+++ b/packages/core-api/src/routing/RouteRefRegistry.test.ts
@@ -75,12 +75,10 @@ describe('RouteRefRegistry', () => {
 
     expect(registry.resolveRoute([], [ref1])).toBe('/1');
     expect(registry.resolveRoute([], [ref2])).toBe('/2');
-    expect(registry.resolveRoute([], [ref2a.link({}), ref1])).toBe('/2/a/1');
-    expect(registry.resolveRoute([], [ref2a.link({}), ref2])).toBe('/2/a/2');
-    expect(registry.resolveRoute([ref2a.link({})], [ref2])).toBe('/2/a/2');
-    expect(registry.resolveRoute([ref2a.link({}), ref1], [ref2])).toBe(
-      '/2/a/2',
-    );
+    expect(registry.resolveRoute([], [ref2a.link(), ref1])).toBe('/2/a/1');
+    expect(registry.resolveRoute([], [ref2a.link(), ref2])).toBe('/2/a/2');
+    expect(registry.resolveRoute([ref2a.link()], [ref2])).toBe('/2/a/2');
+    expect(registry.resolveRoute([ref2a.link(), ref1], [ref2])).toBe('/2/a/2');
     expect(registry.resolveRoute([], [ref2b.link({ id: 'abc' }), ref1])).toBe(
       '/2/b/abc/1',
     );

From d7cd977ce7ed451934ba6dd4f6e9fc634163974b Mon Sep 17 00:00:00 2001
From: Patrik Oldsberg <poldsberg@gmail.com>
Date: Tue, 22 Sep 2020 18:01:04 +0200
Subject: [PATCH 9/9] core-api: add back MutableRouteRef for backwards
 compatibility and export AbsoluteRouteRef

---
 packages/core-api/src/routing/RouteRef.ts | 5 +++++
 packages/core-api/src/routing/index.ts    | 1 +
 2 files changed, 6 insertions(+)

diff --git a/packages/core-api/src/routing/RouteRef.ts b/packages/core-api/src/routing/RouteRef.ts
index c8b0855de7..c33335cb38 100644
--- a/packages/core-api/src/routing/RouteRef.ts
+++ b/packages/core-api/src/routing/RouteRef.ts
@@ -84,3 +84,8 @@ export class AbsoluteRouteRef implements ConcreteRoute {
 export function createRouteRef(config: RouteRefConfig): AbsoluteRouteRef {
   return new AbsoluteRouteRef(config);
 }
+
+// TODO(Rugvip): Added for backwards compatibility, remove once old usage is gone
+// We may want to avoid exporting the AbsoluteRouteRef itself though, and consider
+// a different model for how to create sub routes, just avoid this
+export type MutableRouteRef = AbsoluteRouteRef;
diff --git a/packages/core-api/src/routing/index.ts b/packages/core-api/src/routing/index.ts
index 1b80c0838e..29de34ec42 100644
--- a/packages/core-api/src/routing/index.ts
+++ b/packages/core-api/src/routing/index.ts
@@ -15,4 +15,5 @@
  */
 
 export type { RouteRef, RouteRefConfig, ConcreteRoute } from './types';
+export type { MutableRouteRef, AbsoluteRouteRef } from './RouteRef';
 export { createRouteRef } from './RouteRef';