From 92f96690954200d45a09719c23d07d0646bf5774 Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Fri, 17 Feb 2023 11:01:04 -0500 Subject: [PATCH] chore: use map for conflicts, make sure things are in order recieved, update test infra Signed-off-by: Logan McAnsh --- .../remix-dev/__tests__/flat-routes-test.ts | 48 ++++++++----------- packages/remix-dev/config/flat-routes.ts | 25 +++++++--- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/packages/remix-dev/__tests__/flat-routes-test.ts b/packages/remix-dev/__tests__/flat-routes-test.ts index 8bb588e7b84..7a51363f1a6 100644 --- a/packages/remix-dev/__tests__/flat-routes-test.ts +++ b/packages/remix-dev/__tests__/flat-routes-test.ts @@ -627,8 +627,8 @@ describe("flatRoutes", () => { test("index files", () => { // we'll add file manually before running the tests let testFiles = [ - "routes/_landing._index.tsx", "routes/_dashboard._index.tsx", + "routes/_landing._index.tsx", "routes/_index.tsx", ]; @@ -644,20 +644,13 @@ describe("flatRoutes", () => { // we had a collision as /route and /index are the same expect(routes).toHaveLength(1); expect(consoleError).toHaveBeenCalledWith( - trimAllLines(`⚠️ Route Path Collision: "/" - - The following routes all define the same URL, only the first one will be used - - 🟢 routes${path.sep}_landing._index.tsx - ⭕️️ routes${path.sep}_dashboard._index.tsx - ⭕️️ routes${path.sep}_index.tsx - `) + getErrorMessage("/", testFiles) ); }); test("folder/route.tsx matching folder.tsx", () => { // we'll add file manually before running the tests - let testFiles = ["routes/dashboard/route.tsx", "routes/dashboard.tsx"]; + let testFiles = ["routes/dashboard.tsx", "routes/dashboard/route.tsx"]; let routeManifest = flatRoutesUniversal( APP_DIR, @@ -671,13 +664,7 @@ describe("flatRoutes", () => { // we had a collision as /route and /index are the same expect(routes).toHaveLength(1); expect(consoleError).toHaveBeenCalledWith( - trimAllLines(`⚠️ Route Path Collision: "/dashboard" - - The following routes all define the same URL, only the first one will be used - - 🟢 routes${path.sep}dashboard${path.sep}route.tsx - ⭕️️ routes${path.sep}dashboard.tsx - `) + getErrorMessage("/dashboard", testFiles) ); }); @@ -700,21 +687,24 @@ describe("flatRoutes", () => { // we had a collision as /route and /index are the same expect(routes).toHaveLength(1); expect(consoleError).toHaveBeenCalledWith( - trimAllLines(`⚠️ Route Path Collision: "/products/:productId" - - The following routes all define the same URL, only the first one will be used - - 🟢 routes${path.sep}products.$productId.tsx - ⭕️️ routes${path.sep}products.$pid.tsx - `) + getErrorMessage("/products/:pid", testFiles) ); }); }); }); -function trimAllLines(str: string) { - return str - .split("\n") - .map((line) => line.trim()) - .join("\n"); +function normalizePath(filePath: string) { + return filePath.split("/").join(path.sep); +} + +function getErrorMessage(pathname: string, routes: string[]) { + let [taken, ...others] = routes; + + return ( + `⚠️ Route Path Collision: "${pathname}"\n\n` + + `The following routes all define the same URL, only the first one will be used\n\n` + + `🟢 ${normalizePath(taken)}\n` + + others.map((route) => `⭕️️ ${normalizePath(route)}`).join("\n") + + "\n" + ); } diff --git a/packages/remix-dev/config/flat-routes.ts b/packages/remix-dev/config/flat-routes.ts index b0a4ac3aea9..ffe3e70974b 100644 --- a/packages/remix-dev/config/flat-routes.ts +++ b/packages/remix-dev/config/flat-routes.ts @@ -323,7 +323,7 @@ function getRouteMap( let routeMap = new Map(); let nameMap = new Map(); let uniqueRoutes = new Map(); - let conflicts: { [path: string]: RouteInfo[] } = {}; + let conflicts = new Map(); for (let routePath of routePaths) { let routesDirectory = path.join(appDirectory, prefix); @@ -338,8 +338,14 @@ function getRouteMap( let conflict = uniqueRoutes.get(uniqueRouteId); // collect conflicts for later reporting if (conflict) { - conflicts[routeInfo.path || "/"] ||= [conflict]; - conflicts[routeInfo.path || "/"].push(routeInfo); + let currentConflicts = conflicts.get(routeInfo.path || "/"); + if (!currentConflicts) { + conflicts.set(routeInfo.path || "/", [conflict, routeInfo]); + } else { + currentConflicts.push(routeInfo); + conflicts.set(routeInfo.path || "/", currentConflicts); + } + continue; } uniqueRoutes.set(uniqueRouteId, routeInfo); @@ -377,8 +383,13 @@ function getRouteMap( segment.startsWith(":") && nextSegment.startsWith(":") ) { - conflicts[nextRoute.path || "/"] ||= [nextRoute]; - conflicts[nextRoute.path || "/"].push(routeInfo); + let currentConflicts = conflicts.get(routeInfo.path || "/"); + if (!currentConflicts) { + conflicts.set(routeInfo.path || "/", [routeInfo, nextRoute]); + } else { + currentConflicts.push(nextRoute); + conflicts.set(routeInfo.path || "/", currentConflicts); + } return false; } } @@ -387,8 +398,8 @@ function getRouteMap( }); // report conflicts - if (Object.keys(conflicts).length > 0) { - for (let [path, routes] of Object.entries(conflicts)) { + if (conflicts.size > 0) { + for (let [path, routes] of conflicts.entries()) { let [taken, ...rest] = routes; console.error(