Implement router dynamic route replacement logic (#116)

* Implement router dynamic route replacement logic

Vue router has pretty bad dynamic route registration handlers, meaning we have to hack around replacing the full routes array in order to achieve properly matched routes

* Add test coverage for replacerouter function

* Replace anonymous event handlers with named HoCs for better test coverage

* Add tests for module registration

* Get test coverage to 100%

src/directives/tooltip/tooltip.test.ts

@@ -15,13 +15,21 @@ jest.useFakeTimers();
 const localVue = createLocalVue();
 localVue.use(VueCompositionAPI);
 localVue.component('v-button', VButton);
-localVue.directive('tooltip', Tooltip);
 
 describe('Tooltip', () => {
 	afterEach(() => {
 		document.getElementsByTagName('html')[0].innerHTML = '';
 	});
 
+	describe('Directive', () => {
+		it('Registers onmouseenter and onmouseleave event handlers', () => {
+			const element = document.createElement('div');
+			element.addEventListener = jest.fn();
+			Tooltip.bind!(element, {} as any, null as any, null as any);
+			expect(element.addEventListener).toHaveBeenCalledTimes(2);
+		});
+	});
+
 	describe('onEnterTooltip', () => {
 		it('Instant does not wait to show the tooltip', () => {
 			const div = document.createElement('div');
@@ -35,7 +43,7 @@ describe('Tooltip', () => {
 					top: true,
 					instant: true
 				}
-			});
+			})();
 
 			expect(tooltip.className).toBe('visible enter top');
 		});
@@ -52,7 +60,7 @@ describe('Tooltip', () => {
 					top: true,
 					instant: false
 				}
-			});
+			})();
 
 			expect(tooltip.className).toBe('');
 			jest.advanceTimersByTime(650);
@@ -60,6 +68,13 @@ describe('Tooltip', () => {
 		});
 	});
 
+	describe('onLeaveTooltip', () => {
+		it('Clears the timeout', () => {
+			onLeaveTooltip()();
+			expect(clearTimeout).toHaveBeenCalled();
+		});
+	});
+
 	describe('updateTooltip', () => {
 		describe('Styles and classes', () => {
 			type Modifier = {
@@ -279,13 +294,6 @@ describe('Tooltip', () => {
 		});
 	});
 
-	describe('onLeaveTooltip', () => {
-		it('Clears the timeout', () => {
-			onLeaveTooltip();
-			expect(clearTimeout).toHaveBeenCalled();
-		});
-	});
-
 	describe('animateIn / animateOut', () => {
 		it('Adds the appropriate classes on entering', () => {
 			const div = document.createElement('div');

src/directives/tooltip/tooltip.ts

@@ -2,9 +2,9 @@ import { DirectiveOptions } from 'vue';
 import { DirectiveBinding } from 'vue/types/options';
 
 const Tooltip: DirectiveOptions = {
-	inserted(element, binding) {
-		element.onmouseenter = () => onEnterTooltip(element, binding);
-		element.onmouseleave = () => onLeaveTooltip();
+	bind(element, binding) {
+		element.addEventListener('onmouseenter', onEnterTooltip(element, binding));
+		element.addEventListener('onmouseleave', onLeaveTooltip());
 	}
 };
 
@@ -13,17 +13,28 @@ export default Tooltip;
 let tooltipTimer: number;
 
 export function onEnterTooltip(element: HTMLElement, binding: DirectiveBinding) {
-	const tooltip = getTooltip();
+	return () => {
+		const tooltip = getTooltip();
 
-	if (binding.modifiers.instant) {
-		animateIn(tooltip);
-		updateTooltip(element, binding, tooltip);
-	} else {
-		tooltipTimer = setTimeout(() => {
+		if (binding.modifiers.instant) {
 			animateIn(tooltip);
 			updateTooltip(element, binding, tooltip);
-		}, 600);
-	}
+		} else {
+			tooltipTimer = setTimeout(() => {
+				animateIn(tooltip);
+				updateTooltip(element, binding, tooltip);
+			}, 600);
+		}
+	};
+}
+
+export function onLeaveTooltip() {
+	return () => {
+		const tooltip = getTooltip();
+
+		clearTimeout(tooltipTimer);
+		animateOut(tooltip);
+	};
 }
 
 export function updateTooltip(
@@ -119,13 +130,6 @@ export function updateTooltip(
 	}
 }
 
-export function onLeaveTooltip() {
-	const tooltip = getTooltip();
-
-	clearTimeout(tooltipTimer);
-	animateOut(tooltip);
-}
-
 export function animateIn(tooltip: HTMLElement) {
 	tooltip.classList.add('visible', 'enter');
 	tooltip.classList.remove('leave', 'leave-active');

src/main.ts

@@ -5,10 +5,12 @@ import './plugins';
 import './directives/register';
 import './components/register';
 import './views/register';
-
+import { registerGlobalModules } from './modules/register';
 import router from './router';
 import i18n from './lang/';
 
+registerGlobalModules();
+
 Vue.config.productionTip = false;
 
 const app = new Vue({

src/modules/collections/collections.vue

@@ -1,5 +1,15 @@
 <template>
-	<h1>Collections</h1>
+	<private-view>
+		<template #navigation>
+			Nav
+		</template>
+
+		Collections
+
+		<template #drawer>
+			Drawer
+		</template>
+	</private-view>
 </template>
 
 <script lang="ts">

src/modules/files/files.vue

@@ -1,5 +1,7 @@
 <template>
-	<h1>Files</h1>
+	<private-view>
+		Files
+	</private-view>
 </template>
 
 <script lang="ts">

src/modules/index.ts

@@ -1,6 +0,0 @@
-import CollectionsModule from './collections/';
-import FilesModule from './files/';
-import SettingsModule from './settings/';
-import UsersModule from './users/';
-
-export default [CollectionsModule, FilesModule, SettingsModule, UsersModule];

src/modules/register.test.ts

@@ -0,0 +1,146 @@
+import Vue from 'vue';
+import VueCompositionAPI from '@vue/composition-api';
+import { RouteConfig } from 'vue-router';
+import * as router from '@/router';
+import moduleRegistration from './register';
+import { useModulesStore } from '@/stores/modules';
+import { ModuleConfig } from '@/types/modules';
+
+describe('Modules / Register', () => {
+	beforeAll(() => {
+		Vue.config.productionTip = false;
+		Vue.config.devtools = false;
+		Vue.use(VueCompositionAPI);
+	});
+
+	afterEach(() => {
+		jest.restoreAllMocks();
+	});
+
+	it('Calls replaceRoutes', () => {
+		jest.spyOn(router, 'replaceRoutes');
+		const testModules: ModuleConfig[] = [
+			{
+				id: 'test',
+				icon: 'box',
+				name: 'Test',
+				routes: [
+					{
+						path: '/path'
+					}
+				]
+			}
+		];
+
+		moduleRegistration.registerModules(testModules);
+
+		expect(router.replaceRoutes).toHaveBeenCalled();
+	});
+
+	it('Adds the modules to the store', () => {
+		const modulesStore = useModulesStore({});
+
+		const testModules: ModuleConfig[] = [
+			{
+				id: 'test',
+				icon: 'box',
+				name: 'Test',
+				routes: [
+					{
+						path: '/path'
+					}
+				]
+			}
+		];
+
+		moduleRegistration.registerModules(testModules);
+
+		expect(modulesStore.state.modules).toEqual([
+			{
+				id: 'test',
+				icon: 'box',
+				name: 'Test'
+			}
+		]);
+	});
+
+	it('Calls the name function if name is a method', () => {
+		const testModules: ModuleConfig[] = [
+			{
+				id: 'test',
+				icon: 'box',
+				name: jest.fn(),
+				routes: [
+					{
+						path: '/path'
+					}
+				]
+			}
+		];
+
+		moduleRegistration.registerModules(testModules);
+
+		expect(testModules[0].name).toHaveBeenCalled();
+	});
+
+	it('Calls insertBeforeProjectWildcard on register', () => {
+		const spy = jest.spyOn(moduleRegistration, 'insertBeforeProjectWildcard');
+
+		const testModules: ModuleConfig[] = [
+			{
+				id: 'test',
+				icon: 'box',
+				name: 'Test',
+				routes: [
+					{
+						path: '/path'
+					}
+				]
+			}
+		];
+
+		moduleRegistration.registerModules(testModules);
+
+		expect(moduleRegistration.insertBeforeProjectWildcard).toHaveBeenCalled();
+
+		spy.mockReset();
+	});
+
+	it('Calls registerModule for all global modules', () => {
+		const spy = jest.spyOn(moduleRegistration, 'registerModules');
+		moduleRegistration.registerGlobalModules();
+		expect(moduleRegistration.registerModules).toHaveBeenCalled();
+	});
+
+	it('Inserts the passed routes into the right location in the routes array', () => {
+		const testRoutes: RouteConfig[] = [
+			{
+				path: '/'
+			},
+			// Routes should be inserted here
+			{
+				path: '/:project/*'
+			}
+		];
+
+		const testModules: RouteConfig[] = [
+			{
+				path: '/:project/test/path'
+			}
+		];
+
+		const newRoutes = moduleRegistration.insertBeforeProjectWildcard(testRoutes, testModules);
+
+		expect(newRoutes).toEqual([
+			{
+				path: '/'
+			},
+			{
+				path: '/:project/test/path'
+			},
+			{
+				path: '/:project/*'
+			}
+		]);
+	});
+});

src/modules/register.ts

@@ -0,0 +1,66 @@
+import CollectionsModule from './collections/';
+import FilesModule from './files/';
+import SettingsModule from './settings/';
+import UsersModule from './users/';
+import { RouteConfig } from 'vue-router';
+import { ModuleConfig, Module } from '@/types/modules';
+import { replaceRoutes } from '@/router';
+import useModulesStore from '@/stores/modules';
+import { i18n } from '@/lang';
+
+const lib = {
+	registerGlobalModules,
+	registerModules,
+	insertBeforeProjectWildcard
+};
+
+export default lib;
+
+export function insertBeforeProjectWildcard(routes: RouteConfig[], moduleRoutes: RouteConfig[]) {
+	// Find the index of the /:project/* route, so we can insert the module routes right above that
+	const wildcardIndex = routes.findIndex(route => route.path === '/:project/*');
+	return [...routes.slice(0, wildcardIndex), ...moduleRoutes, ...routes.slice(wildcardIndex)];
+}
+
+export function registerModules(modules: ModuleConfig[]) {
+	const modulesStore = useModulesStore();
+
+	/** @todo
+	 * This is where we will download the module definitions for custom modules
+	 */
+
+	const moduleRoutes: RouteConfig[] = modules
+		.map(config => {
+			// Prefix all module paths with /:project/:module
+			return config.routes.map(route => ({
+				...route,
+				path: `/:project/${config.id}${route.path}`
+			}));
+		})
+		.flat();
+
+	replaceRoutes(routes => lib.insertBeforeProjectWildcard(routes, moduleRoutes));
+
+	const modulesForStore: Module[] = modules.map(config => {
+		const name = typeof config.name === 'function' ? config.name(i18n) : config.name;
+
+		return {
+			id: config.id,
+			name: name,
+			icon: config.icon
+		};
+	});
+
+	modulesStore.state.modules = modulesForStore;
+}
+
+export function registerGlobalModules() {
+	const globalModules: ModuleConfig[] = [
+		CollectionsModule,
+		FilesModule,
+		SettingsModule,
+		UsersModule
+	];
+
+	lib.registerModules(globalModules);
+}

src/modules/settings/settings.vue

@@ -1,5 +1,7 @@
 <template>
-	<h1>Settings</h1>
+	<private-view>
+		Settings
+	</private-view>
 </template>
 
 <script lang="ts">

src/modules/users/users.vue

@@ -1,5 +1,7 @@
 <template>
-	<h1>Users</h1>
+	<private-view>
+		Users
+	</private-view>
 </template>
 
 <script lang="ts">

src/router.test.ts

@@ -1,10 +1,14 @@
 import Vue from 'vue';
 import VueCompositionAPI from '@vue/composition-api';
-import VueRouter, { Route } from 'vue-router';
-import { onBeforeEach, onBeforeEnterProjectChooser } from './router';
+import { Route } from 'vue-router';
+import router, {
+	onBeforeEach,
+	onBeforeEnterProjectChooser,
+	replaceRoutes,
+	defaultRoutes
+} from './router';
 import { useProjectsStore } from '@/stores/projects';
 import api from '@/api';
-import useModulesStore from './stores/modules';
 import * as auth from '@/auth';
 
 const route: Route = {
@@ -47,22 +51,6 @@ describe('Router', () => {
 		expect(projectsStore.getProjects).toHaveBeenCalled();
 	});
 
-	it('Registers all global modules on first load', async () => {
-		const modulesStore = useModulesStore({});
-		modulesStore.registerGlobalModules = jest.fn();
-
-		const toRoute = route;
-		const fromRoute = {
-			...route,
-			name: null
-		};
-		const callback = jest.fn();
-
-		await onBeforeEach(toRoute, fromRoute as any, callback);
-
-		expect(modulesStore.registerGlobalModules).toHaveBeenCalled();
-	});
-
 	it('Redirects to /install if projectsStore indicates that an install is needed', async () => {
 		const projectsStore = useProjectsStore({});
 		projectsStore.state.needsInstall = true;
@@ -234,4 +222,12 @@ describe('Router', () => {
 			expect(projectsStore.state.currentProjectKey).toBe(null);
 		});
 	});
+
+	describe('replaceRoutes', () => {
+		it('Calls the handler with the default routes', async () => {
+			const handler = jest.fn(() => []);
+			replaceRoutes(handler);
+			expect(handler).toHaveBeenCalledWith(defaultRoutes);
+		});
+	});
 });

src/router.ts

@@ -1,7 +1,6 @@
-import VueRouter, { NavigationGuard } from 'vue-router';
+import VueRouter, { NavigationGuard, RouteConfig } from 'vue-router';
 import Debug from '@/routes/debug.vue';
 import { useProjectsStore } from '@/stores/projects';
-import { useModulesStore } from '@/stores/modules';
 import LoginRoute from '@/routes/login';
 import ProjectChooserRoute from '@/routes/project-chooser';
 import { checkAuth } from '@/auth';
@@ -12,48 +11,72 @@ export const onBeforeEnterProjectChooser: NavigationGuard = (to, from, next) =>
 	next();
 };
 
+export const defaultRoutes: RouteConfig[] = [
+	{
+		path: '/',
+		component: ProjectChooserRoute,
+		meta: {
+			public: true
+		},
+		beforeEnter: onBeforeEnterProjectChooser
+	},
+	{
+		path: '/install',
+		component: LoginRoute,
+		meta: {
+			public: true
+		}
+	},
+	{
+		path: '/:project',
+		redirect: '/:project/login'
+	},
+	{
+		path: '/:project/login',
+		component: LoginRoute,
+		meta: {
+			public: true
+		}
+	},
+	/**
+	 * @NOTE
+	 * Dynamic modules need to be inserted here. By default, VueRouter.addRoutes adds the route
+	 * to the end of this list, meaning that the Private404 will match before the custom module..
+	 * Vue Router dynamic route registration is under discussion:
+	 * https://github.com/vuejs/vue-router/issues/1156, and has an RFC:
+	 * https://github.com/vuejs/rfcs/pull/122
+	 *
+	 * In order to achieve what we need, we can use the custom replaceRoutes function exported
+	 * below to replace all the routes. This allows us to override this list of routes with the
+	 * list augmented with the module routes in the correct location.
+	 */
+	{
+		path: '/:project/*',
+		// This will be Private404
+		component: Debug
+	},
+	{
+		path: '*',
+		// This will be Public404
+		component: Debug
+	}
+];
+
 const router = new VueRouter({
 	mode: 'hash',
-	routes: [
-		{
-			path: '/',
-			component: ProjectChooserRoute,
-			meta: {
-				public: true
-			},
-			beforeEnter: onBeforeEnterProjectChooser
-		},
-		{
-			path: '/install',
-			component: LoginRoute,
-			meta: {
-				public: true
-			}
-		},
-		{
-			path: '/:project',
-			redirect: '/:project/login'
-		},
-		{
-			path: '/:project/login',
-			component: LoginRoute,
-			meta: {
-				public: true
-			}
-		},
-		/** @NOTE
-		 * All modules are registered dynamically as `/:project/:module`
-		 */
-		{
-			path: '/:project/*',
-			component: Debug
-		}
-	]
+	routes: defaultRoutes
 });
 
+export function replaceRoutes(routeFilter: (routes: RouteConfig[]) => RouteConfig[]): void {
+	const newRoutes = routeFilter([...defaultRoutes]);
+	const newRouter = new VueRouter({ routes: newRoutes });
+
+	// @ts-ignore - Matcher is not officially part of the public API (https://github.com/vuejs/vue-router/issues/2844#issuecomment-509529927)
+	router.matcher = newRouter.matcher;
+}
+
 export const onBeforeEach: NavigationGuard = async (to, from, next) => {
 	const projectsStore = useProjectsStore();
-	const modulesStore = useModulesStore();
 
 	// Only on first load is from.name null. On subsequent requests, from.name is undefined | string
 	const firstLoad = from.name === null;
@@ -62,7 +85,6 @@ export const onBeforeEach: NavigationGuard = async (to, from, next) => {
 	// platform. We can also use this to (async) register all the globally available modules
 	if (firstLoad) {
 		await projectsStore.getProjects();
-		modulesStore.registerGlobalModules();
 	}
 
 	// When there aren't any projects, we should redirect to the install page to force the

src/stores/modules/modules.test.ts

@@ -1,104 +0,0 @@
-import Vue from 'vue';
-import VueCompositionAPI from '@vue/composition-api';
-import router from '@/router';
-import { useModulesStore } from './modules';
-import { ModuleConfig } from '@/types/modules';
-import systemModules from '@/modules/';
-jest.mock('@/modules/', () => [
-	{
-		id: 'system-test-1',
-		icon: 'box',
-		name: 'System Module 1',
-		routes: []
-	},
-	{
-		id: 'system-test-2',
-		icon: 'box',
-		name: 'System Module 2',
-		routes: []
-	}
-]);
-
-describe('Stores / Modules', () => {
-	beforeAll(() => {
-		Vue.use(VueCompositionAPI);
-	});
-
-	describe('registerModule', () => {
-		it('Prefixes each route path with /:project/:module/', () => {
-			jest.spyOn(router, 'addRoutes');
-			const modulesStore = useModulesStore({});
-			const testModule: ModuleConfig = {
-				id: 'test-module',
-				icon: 'box',
-				name: 'Test Module',
-				routes: [
-					{
-						path: '/test'
-					}
-				]
-			};
-
-			modulesStore.registerModule(testModule);
-			expect(router.addRoutes).toHaveBeenCalledWith([
-				{
-					path: '/:project/test-module/test'
-				}
-			]);
-		});
-
-		it('Adds the routes when registering the module', () => {
-			jest.spyOn(router, 'addRoutes');
-			const modulesStore = useModulesStore({});
-			const testModule: ModuleConfig = {
-				id: 'test-module',
-				icon: 'box',
-				name: 'Test Module',
-				routes: []
-			};
-			modulesStore.registerModule(testModule);
-			expect(router.addRoutes).toHaveBeenCalledWith([]);
-		});
-
-		it('Adds the module to the store if registration is completed', () => {
-			jest.spyOn(router, 'addRoutes');
-			const modulesStore = useModulesStore({});
-			const testModule: ModuleConfig = {
-				id: 'test-module',
-				icon: 'box',
-				name: 'Test Module',
-				routes: []
-			};
-			modulesStore.registerModule(testModule);
-			expect(modulesStore.state.modules).toEqual([
-				{
-					id: 'test-module',
-					icon: 'box',
-					name: 'Test Module'
-				}
-			]);
-		});
-
-		it('Uses the static name, or calls the function if method is given for name', () => {
-			jest.spyOn(router, 'addRoutes');
-			const modulesStore = useModulesStore({});
-			const testModule: ModuleConfig = {
-				id: 'test-module',
-				icon: 'box',
-				name: jest.fn(),
-				routes: []
-			};
-			modulesStore.registerModule(testModule);
-			expect(testModule.name).toHaveBeenCalled();
-		});
-	});
-
-	describe('registerGlobalModules', () => {
-		it('Calls registerModule for each global module', () => {
-			const modulesStore = useModulesStore({});
-			modulesStore.registerModule = jest.fn();
-			modulesStore.registerGlobalModules();
-			expect(modulesStore.registerModule).toHaveBeenCalledTimes(2);
-		});
-	});
-});

src/stores/modules/modules.ts

@@ -1,44 +1,9 @@
 import { createStore } from 'pinia';
-import { Module, ModuleConfig } from '@/types/modules';
-import systemModules from '@/modules/';
-import router from '@/router';
-import { i18n } from '@/lang';
+import { Module } from '@/types/modules';
 
 export const useModulesStore = createStore({
 	id: 'modules',
 	state: () => ({
 		modules: [] as Module[]
-	}),
-	actions: {
-		/**
-		 * Registers all global modules, including the system ones
-		 */
-		registerGlobalModules() {
-			systemModules.forEach(this.registerModule);
-		},
-		/**
-		 * Register a single module. Adds the routes to the router, resolves the name, and adds it to
-		 * the modules store state.
-		 * @param config Config object for module
-		 */
-		registerModule(config: ModuleConfig) {
-			const routes = config.routes.map(route => ({
-				...route,
-				path: `/:project/${config.id}${route.path}`
-			}));
-
-			router.addRoutes(routes);
-
-			const name = typeof config.name === 'function' ? config.name(i18n) : config.name;
-
-			this.state.modules = [
-				...this.state.modules,
-				{
-					id: config.id,
-					icon: config.icon,
-					name: name
-				}
-			];
-		}
-	}
+	})
 });

src/views/private/private-view.vue

@@ -2,7 +2,9 @@
 	<div class="private-view">
 		<aside class="navigation" :class="{ 'is-open': navOpen }">
 			<module-bar />
-			<div class="module-nav"></div>
+			<div class="module-nav">
+				<slot name="navigation" />
+			</div>
 		</aside>
 		<div class="content">
 			<header>
@@ -11,7 +13,9 @@
 			</header>
 			<main><slot /></main>
 		</div>
-		<aside class="drawer" :class="{ 'is-open': drawerOpen }"></aside>
+		<aside class="drawer" :class="{ 'is-open': drawerOpen }">
+			<slot name="drawer" />
+		</aside>
 
 		<v-overlay
 			v-if="navWithOverlay"