remix-run · ryanflorence · Feb 21, 2023 · Feb 15, 2023 · Feb 15, 2023 · Feb 15, 2023
diff --git a/.changeset/flat-routes-route-fixes.md b/.changeset/flat-routes-route-fixes.md
@@ -0,0 +1,34 @@
+---
+"remix": patch
+"@remix-run/dev": patch
+---
+
+fixes flat route inconsistencies where `route.{ext}` wasn't always being treated like `index.{ext}` when used in a folder
+
+route conflict no longer throw errors and instead display a helpful warning that we're using the first one we found.
+
+```log
+⚠️ Route Path Collision: "/products/:pid"
+
+The following routes all define the same URL, only the first one will be used
+
+🟢️️ routes/products.$pid.tsx
+⭕️️ routes/products.$productId.tsx
+```
+```log
+⚠️ Route Path Collision: "/dashboard"
+
+The following routes all define the same URL, only the first one will be used
+
+🟢️️ routes/dashboard/route.tsx
+⭕️️ routes/dashboard.tsx
+```
+```log
+⚠️ Route Path Collision: "/"
+
+The following routes all define the same URL, only the first one will be used
+
+🟢️️ routes/_landing._index.tsx
+⭕️️ routes/_dashboard._index.tsx
+⭕️ routes/_index.tsx
+```
diff --git a/integration/flat-routes-test.ts b/integration/flat-routes-test.ts
@@ -0,0 +1,204 @@
+import { PassThrough } from "node:stream";
+import { test, expect } from "@playwright/test";
+
+import { PlaywrightFixture } from "./helpers/playwright-fixture";
+import type { Fixture, AppFixture } from "./helpers/create-fixture";
+import { createFixtureProject } from "./helpers/create-fixture";
+import { createAppFixture, createFixture, js } from "./helpers/create-fixture";
+
+let fixture: Fixture;
+let appFixture: AppFixture;
+
+test.describe("flat routes", () => {
+  test.beforeAll(async () => {
+    fixture = await createFixture({
+      future: { v2_routeConvention: true },
+      files: {
+        "app/root.jsx": js`
+          import { Links, Meta, Outlet, Scripts } from "@remix-run/react";
+
+          export default function Root() {
+            return (
+              <html lang="en">
+                <head>
+                  <Meta />
+                  <Links />
+                </head>
+                <body>
+                  <div id="content">
+                    <h1>Root</h1>
+                    <Outlet />
+                  </div>
+                  <Scripts />
+                </body>
+              </html>
+            );
+          }
+        `,
+
+        "app/routes/_index.jsx": js`
+          export default function () {
+            return <h2>Index</h2>;
+          }
+        `,
+
+        "app/routes/folder/route.jsx": js`
+          export default function () {
+            return <h2>Folder (Route.jsx)</h2>;
+          }
+        `,
+
+        "app/routes/folder2/index.jsx": js`
+          export default function () {
+            return <h2>Folder (Index.jsx)</h2>;
+          }
+        `,
+
+        "app/routes/flat.file.jsx": js`
+          export default function () {
+            return <h2>Flat File</h2>;
+          }
+        `,
+
+        "app/routes/dashboard/route.jsx": js`
+          import { Outlet } from "@remix-run/react";
+
+          export default function () {
+            return (
+              <>
+                <h2>Dashboard Layout</h2>
+                <Outlet />
+              </>
+            )
+          }
+        `,
+
+        "app/routes/dashboard._index/route.jsx": js`
+          export default function () {
+            return <h3>Dashboard Index</h3>;
+          }
+        `,
+      },
+    });
+
+    appFixture = await createAppFixture(fixture);
+  });
+
+  test.afterAll(() => {
+    appFixture.close();
+  });
+
+  test.describe("without JavaScript", () => {
+    test.use({ javaScriptEnabled: false });
+    runTests();
+  });
+
+  test.describe("with JavaScript", () => {
+    test.use({ javaScriptEnabled: true });
+    runTests();
+  });
+
+  function runTests() {
+    test("renders matching routes (index)", async ({ page }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/");
+      expect(await app.getHtml("#content")).toBe(`<div id="content">
+  <h1>Root</h1>
+  <h2>Index</h2>
+</div>`);
+    });
+
+    test("renders matching routes (folder route.jsx)", async ({ page }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/folder");
+      expect(await app.getHtml("#content")).toBe(`<div id="content">
+  <h1>Root</h1>
+  <h2>Folder (Route.jsx)</h2>
+</div>`);
+    });
+
+    test("renders matching routes (folder index.jsx)", async ({ page }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/folder2");
+      expect(await app.getHtml("#content")).toBe(`<div id="content">
+  <h1>Root</h1>
+  <h2>Folder (Index.jsx)</h2>
+</div>`);
+    });
+
+    test("renders matching routes (flat file)", async ({ page }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/flat/file");
+      expect(await app.getHtml("#content")).toBe(`<div id="content">
+  <h1>Root</h1>
+  <h2>Flat File</h2>
+</div>`);
+    });
+
+    test("renders matching routes (nested)", async ({ page }) => {
+      let app = new PlaywrightFixture(appFixture, page);
+      await app.goto("/dashboard");
+      expect(await app.getHtml("#content")).toBe(`<div id="content">
+  <h1>Root</h1>
+  <h2>Dashboard Layout</h2>
+  <h3>Dashboard Index</h3>
+</div>`);
+    });
+  }
+});
+
+test.describe("emits warnings for route conflicts", async () => {
+  let buildStdio = new PassThrough();
+  let buildOutput: string;
+
+  let originalConsoleLog = console.log;
+  let originalConsoleWarn = console.warn;
+  let originalConsoleError = console.error;
+
+  test.beforeAll(async () => {
+    console.log = () => {};
+    console.warn = () => {};
+    console.error = () => {};
+    await createFixtureProject({
+      buildStdio,
+      future: { v2_routeConvention: true },
+      files: {
+        "routes/_dashboard._index.tsx": js`
+          export default function () {
+            return <p>routes/_dashboard._index</p>;
+          }
+        `,
+        "app/routes/_index.jsx": js`
+          export default function () {
+            return <p>routes._index</p>;
+          }
+        `,
+        "app/routes/_landing._index.jsx": js`
+          export default function () {
+            return <p>routes/_landing._index</p>;
+          }
+        `,
+      },
+    });
+
+    let chunks: Buffer[] = [];
+    buildOutput = await new Promise<string>((resolve, reject) => {
+      buildStdio.on("data", (chunk) => chunks.push(Buffer.from(chunk)));
+      buildStdio.on("error", (err) => reject(err));
+      buildStdio.on("end", () =>
+        resolve(Buffer.concat(chunks).toString("utf8"))
+      );
+    });
+  });
+
+  test.afterAll(() => {
+    console.log = originalConsoleLog;
+    console.warn = originalConsoleWarn;
+    console.error = originalConsoleError;
+  });
+
+  test("warns about conflicting routes", () => {
+    console.log(buildOutput);
+    expect(buildOutput).toContain(`⚠️ Route Path Collision: "/"`);
+  });
+});
diff --git a/packages/remix-dev/__tests__/flat-routes-test.ts b/packages/remix-dev/__tests__/flat-routes-test.ts
@@ -1,10 +1,11 @@
 import path from "node:path";
 
 import {
-  createRoutePath,
   flatRoutesUniversal,
+  getRouteConflictErrorMessage,
+  getRouteInfo,
   getRouteSegments,
-  isIndexRoute,
+  normalizePath,
 } from "../config/flat-routes";
 import type { ConfigRoute } from "../config/routes";
 
@@ -88,11 +89,9 @@ describe("flatRoutes", () => {
 
     for (let [input, expected] of tests) {
       it(`"${input}" -> "${expected}"`, () => {
-        let isIndex = isIndexRoute(input);
-        let [routeSegments, rawRouteSegments] = getRouteSegments(input);
-        expect(createRoutePath(routeSegments, rawRouteSegments, isIndex)).toBe(
-          expected
-        );
+        let fullRoutePath = path.join(APP_DIR, "routes", `${input}.tsx`);
+        let routeInfo = getRouteInfo(APP_DIR, "routes", fullRoutePath);
+        expect(routeInfo.path).toBe(expected);
       });
     }
 
@@ -276,7 +275,7 @@ describe("flatRoutes", () => {
       [
         "routes/folder/route.tsx",
         {
-          id: "routes/folder/route",
+          id: "routes/folder",
           parentId: "root",
           path: "folder",
         },
@@ -601,7 +600,7 @@ describe("flatRoutes", () => {
     ];
 
     let files: [string, ConfigRoute][] = testFiles.map(([file, route]) => {
-      let filepath = file.split("/").join(path.sep);
+      let filepath = normalizePath(file);
       return [filepath, { ...route, file: filepath }];
     });
 
@@ -619,4 +618,73 @@ describe("flatRoutes", () => {
       });
     }
   });
+
+  describe("warns when there's a route collision", () => {
+    let consoleError = jest
+      .spyOn(global.console, "error")
+      .mockImplementation(() => {});
+
+    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",
+      ];
+
+      let routeManifest = flatRoutesUniversal(
+        APP_DIR,
+        testFiles.map((file) => path.join(APP_DIR, normalizePath(file)))
+      );
+
+      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)
+      );
+    });
+
+    test("folder/route.tsx matching folder.tsx", () => {
+      // we'll add file manually before running the tests
+      let testFiles = ["routes/dashboard.tsx", "routes/dashboard/route.tsx"];
+
+      let routeManifest = flatRoutesUniversal(
+        APP_DIR,
+        testFiles.map((file) => path.join(APP_DIR, normalizePath(file)))
+      );
+
+      let routes = Object.values(routeManifest);
+
+      // we had a collision as /route and /index are the same
+      expect(routes).toHaveLength(1);
+      expect(consoleError).toHaveBeenCalledWith(
+        getRouteConflictErrorMessage("/dashboard", testFiles)
+      );
+    });
+
+    test("same path, different param name", () => {
+      // we'll add file manually before running the tests
+      let testFiles = [
+        "routes/products.$pid.tsx",
+        "routes/products.$productId.tsx",
+      ];
+
+      let routeManifest = flatRoutesUniversal(
+        APP_DIR,
+        testFiles.map((file) => path.join(APP_DIR, normalizePath(file)))
+      );
+
+      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)
+      );
+    });
+  });
 });