From 6692e178d1461a1962d65d7b4f6b6c50b1c136fe Mon Sep 17 00:00:00 2001 From: Logan McAnsh Date: Thu, 9 Mar 2023 12:05:21 -0500 Subject: [PATCH] chore: rename a few things, make tests work on windows Signed-off-by: Logan McAnsh --- .../remix-dev/__tests__/flat-routes-test.ts | 35 ++++++---------- packages/remix-dev/config/flat-routes.ts | 41 ++++++++----------- 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/packages/remix-dev/__tests__/flat-routes-test.ts b/packages/remix-dev/__tests__/flat-routes-test.ts index 7234f9a9eeb..51f1dc9326b 100644 --- a/packages/remix-dev/__tests__/flat-routes-test.ts +++ b/packages/remix-dev/__tests__/flat-routes-test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { flatRoutesUniversal, - getRouteConflictErrorMessage, + getRoutePathConflictErrorMessage, getRouteIdConflictErrorMessage, getRouteSegments, } from "../config/flat-routes"; @@ -647,11 +647,10 @@ describe("flatRoutes", () => { afterEach(consoleError.mockReset); test("index files", () => { - // we'll add file manually before running the tests let testFiles = [ - "routes/_dashboard._index.tsx", - "routes/_landing._index.tsx", - "routes/_index.tsx", + path.join("routes", "_dashboard._index.tsx"), + path.join("routes", "_landing._index.tsx"), + path.join("routes", "_index.tsx"), ]; let routeManifest = flatRoutesUniversal( @@ -661,52 +660,44 @@ describe("flatRoutes", () => { let routes = Object.values(routeManifest); - // we had a collision as /route and /index are the same expect(routes).toHaveLength(1); expect(consoleError).toHaveBeenCalledWith( - getRouteConflictErrorMessage("/", testFiles) + getRoutePathConflictErrorMessage("/", testFiles) ); }); test("folder/route.tsx matching folder.tsx", () => { - // we'll add file manually before running the tests let testFiles = [ - path.join(APP_DIR, "routes/dashboard/route.tsx"), - path.join(APP_DIR, "routes/dashboard.tsx"), + path.join(APP_DIR, "routes", "dashboard", "route.tsx"), + path.join(APP_DIR, "routes", "dashboard.tsx"), ]; let routeManifest = flatRoutesUniversal(APP_DIR, testFiles); let routes = Object.values(routeManifest); - // we had a collision as /route and /index are the same expect(routes).toHaveLength(1); expect(consoleError).toHaveBeenCalledWith( getRouteIdConflictErrorMessage( - "routes/dashboard", - testFiles.map((file) => path.relative(APP_DIR, file)) + path.join("routes", "dashboard"), + testFiles ) ); }); test.skip("same path, different param name", () => { - // we'll add file manually before running the tests let testFiles = [ - "routes/products.$pid.tsx", - "routes/products.$productId.tsx", + path.join(APP_DIR, "routes", "products.$pid.tsx"), + path.join(APP_DIR, "routes", "products.$productId.tsx"), ]; - let routeManifest = flatRoutesUniversal( - APP_DIR, - testFiles.map((file) => path.join(APP_DIR, file)) - ); + let routeManifest = flatRoutesUniversal(APP_DIR, testFiles); let routes = Object.values(routeManifest); - // we had a collision as /route and /index are the same expect(routes).toHaveLength(1); expect(consoleError).toHaveBeenCalledWith( - getRouteConflictErrorMessage("/products/:pid", testFiles) + getRoutePathConflictErrorMessage("/products/:pid", testFiles) ); }); }); diff --git a/packages/remix-dev/config/flat-routes.ts b/packages/remix-dev/config/flat-routes.ts index 8dfc4048949..a9cd7686f01 100644 --- a/packages/remix-dev/config/flat-routes.ts +++ b/packages/remix-dev/config/flat-routes.ts @@ -129,7 +129,7 @@ export function flatRoutesUniversal( routes: string[], prefix: string = "routes" ): RouteManifest { - let conflicts = new Map(); + let urlConflicts = new Map(); let routeManifest: RouteManifest = {}; let prefixLookup = new PrefixLookupTrie(); let uniqueRoutes = new Map(); @@ -148,10 +148,8 @@ export function flatRoutesUniversal( let conflict = routeIds.get(routeId); if (conflict) { - let currentConflicts = routeIdConflicts.get(routeId) || [ - path.relative(appDirectory, conflict), - ]; - currentConflicts.push(path.relative(appDirectory, file)); + let currentConflicts = routeIdConflicts.get(routeId) || [conflict]; + currentConflicts.push(file); routeIdConflicts.set(routeId, currentConflicts); continue; } @@ -216,10 +214,10 @@ export function flatRoutesUniversal( uniqueRoutes.set(conflictRouteId, config); if (conflict && (originalPathname || config.index)) { - let currentConflicts = conflicts.get(originalPathname); + let currentConflicts = urlConflicts.get(originalPathname); if (!currentConflicts) currentConflicts = [conflict]; currentConflicts.push(config); - conflicts.set(originalPathname, currentConflicts); + urlConflicts.set(originalPathname, currentConflicts); continue; } } @@ -231,17 +229,14 @@ export function flatRoutesUniversal( } // report conflicts - if (conflicts.size > 0) { - for (let [path, routes] of conflicts.entries()) { + if (urlConflicts.size > 0) { + for (let [path, routes] of urlConflicts.entries()) { + // delete all but the first route from the manifest for (let i = 1; i < routes.length; i++) { delete routeManifest[routes[i].id]; } - console.error( - getRouteConflictErrorMessage( - path, - routes.map((r) => r.file) - ) - ); + let files = routes.map((r) => r.file); + console.error(getRoutePathConflictErrorMessage(path, files)); } } @@ -276,9 +271,9 @@ function findRouteModuleForFolder( ); let routePath = createRoutePath(segments, raw, false); console.error( - getRouteConflictErrorMessage(routePath || "/", [ - path.relative(appDirectory, routeRouteModule), - path.relative(appDirectory, routeIndexModule), + getRoutePathConflictErrorMessage(routePath || "/", [ + routeRouteModule, + routeIndexModule, ]) ); } @@ -458,18 +453,18 @@ export function createRoutePath( return result.length ? result.join("/") : undefined; } -export function getRouteConflictErrorMessage( +export function getRoutePathConflictErrorMessage( pathname: string, routes: string[] ) { let [taken, ...others] = routes; - let pathnameWithLeadingSlash = pathname.startsWith("/") - ? pathname - : "/" + pathname; + if (!pathname.startsWith("/")) { + pathname = "/" + pathname; + } return ( - `⚠️ Route Path Collision: "${pathnameWithLeadingSlash}"\n\n` + + `⚠️ Route Path Collision: "${pathname}"\n\n` + `The following routes all define the same URL, only the first one will be used\n\n` + `🟢 ${taken}\n` + others.map((route) => `⭕️️ ${route}`).join("\n") +