From 3a5b0197e3e5b41bb868803c4a3e0c378e323e8d Mon Sep 17 00:00:00 2001 From: Jeremy Weinstein Date: Mon, 22 May 2023 11:15:38 -0700 Subject: [PATCH] fix: generating routes in parallel --- packages/amagaki/src/router.test.ts | 15 ++++++++----- packages/amagaki/src/router.ts | 33 ++++++++++++++++++----------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/packages/amagaki/src/router.test.ts b/packages/amagaki/src/router.test.ts index 9bcef8ac..bc92c106 100644 --- a/packages/amagaki/src/router.test.ts +++ b/packages/amagaki/src/router.test.ts @@ -1,6 +1,6 @@ import {ExecutionContext} from 'ava'; import {Pod} from './pod'; -import { Route } from './router'; +import {Route} from './router'; import {Server} from './server'; import getPort from 'get-port'; import test from 'ava'; @@ -49,17 +49,17 @@ test('Changing static files alters the router', async (t: ExecutionContext) => { test('AddRoute', async (t: ExecutionContext) => { const pod = new Pod('./fixtures/sampleRouteProvider/'); - pod.router.addRoutes('default', async (provider) => { + pod.router.addRoutes('default', async provider => { provider.addRoute({ urlPath: '/foo/bar/', build: async () => 'foo', - }) + }); }); - pod.router.addRoutes('custom', async (provider) => { + pod.router.addRoutes('custom', async provider => { provider.addRoute({ urlPath: '/foo/custom/', build: async () => 'custom', - }) + }); }); for (const [path, content] of [ ['/foo/bar/', 'foo'], @@ -70,3 +70,8 @@ test('AddRoute', async (t: ExecutionContext) => { t.deepEqual(content, html); } }); + +test('Parallel route building', async (t: ExecutionContext) => { + const pod = new Pod('./fixtures/sampleRouteProvider/'); + await Promise.all([pod.warmup(), pod.warmup()]); +}); diff --git a/packages/amagaki/src/router.ts b/packages/amagaki/src/router.ts index 78d66e72..21c699f3 100644 --- a/packages/amagaki/src/router.ts +++ b/packages/amagaki/src/router.ts @@ -30,7 +30,6 @@ export interface RouteOptions { fields?: Record; } - export type RouteBuilder = (provider: RouteProvider) => Promise; export class Router { @@ -89,25 +88,33 @@ export class Router { if (this.pod.cache.routes.length) { return this.pod.cache.routes; } + const routes = new Map(); for (const providers of Object.values(this.providers)) { for (const provider of providers) { - const routes = await provider.routes(); - for (const route of routes) { - const routeUrl = route.url.path; - if (routeUrl in this.pod.cache.routeMap) { - // Reset the cache so subsequent requests continue to error until - // the problem is resolved. - const foundRoute = this.pod.cache.routeMap[routeUrl]; + const providerRoutes = await provider.routes(); + for (const route of providerRoutes) { + const urlPath = route.url.path; + if (routes.has(urlPath)) { + // Reset the cache so subsequent requests to the dev server continue + // to show an error until the problem is resolved. this.pod.cache.reset(); + const existingRoute = routes[urlPath]; throw Error( - `Two routes share the same URL path (${route.url.path}): ${foundRoute} and ${route}. This probably means you have set the value for "$path" to the same thing for two different documents, or two locales of the same document. Ensure every route has a unique URL path by changing one of the "$path" values.` + `Two routes share the same URL path (${urlPath}): ${route} and ${existingRoute}. This probably means you have set the value for "$path" to the same thing for two different documents, or two locales of the same document. Ensure every route has a unique URL path by changing one of the "$path" values.` ); } - this.pod.cache.routes.push(route); - this.pod.cache.routeMap[route.url.path] = route; + routes.set(urlPath, route); } } } + this.pod.cache.routes = [ + ...this.pod.cache.routes, + ...Array.from(routes.values()), + ]; + this.pod.cache.routeMap = { + ...this.pod.cache.routeMap, + ...Object.fromEntries(routes), + }; return this.pod.cache.routes; } @@ -182,7 +189,9 @@ export class RouteProvider { return this._routes; } // Build all routes, then clear the route builders so they are not built again. - await Promise.all(this._routeBuilders.map(async (builder) => await builder(this))); + await Promise.all( + this._routeBuilders.map(async builder => await builder(this)) + ); return this._routes; }