chore: rename a few things, make tests work on windows

Signed-off-by: Logan McAnsh <logan@mcan.sh>
remix-run · Mar 9, 2023 · 6692e17 · 6692e17
1 parent 7700efe
commit 6692e17
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 45 deletions.
diff --git 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
@@ -129,7 +129,7 @@ export function flatRoutesUniversal(
   routes: string[],
   prefix: string = "routes"
 ): RouteManifest {
-  let conflicts = new Map<string, ConfigRoute[]>();
+  let urlConflicts = new Map<string, ConfigRoute[]>();
   let routeManifest: RouteManifest = {};
   let prefixLookup = new PrefixLookupTrie();
   let uniqueRoutes = new Map<string, ConfigRoute>();
@@ -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") +