Repository: remix-run/remix
Pull request #5228 (2023-01-23 22:33:50)

fix(remix-dev): flat route fixes and enhancements
Signed-off-by: Logan McAnsh <logan@mcan.sh>

<!--

👋 Hey, thanks for your interest in contributing to Remix!

If this is a simple docs change, go ahead and delete all this.

**Please ask first before starting work on any significant new features.**

It's never a fun experience to have your pull request declined after investing a
lot of time and effort into a new feature. To avoid this from happening, we
request that contributors create a
[Feature Request discussion](https://github.com/remix-run/remix/discussions/new?category=ideas)
to first discuss any significant new features.

https://github.com/remix-run/remix/blob/main/CONTRIBUTING.md

Please fill in or delete each item below:

-->

Closes: #5170
Closes: #5261
Closes: #5303
Closes: #5322
Closes: #5324
Closes: #5327

- [ ] Docs
- [x] Tests

Testing Strategy:

updated current tests

<!--
Please explain how you tested this. For example:

> This test covers this code: <link to test>

Or

> I opened up my windows machine and ran this script:
>
> ```
> npx create-remix@0.0.0-experimental-7e420ee3 --template remix my-test
> cd my-test
> npm run dev
> ```
-->


changeset-bot[bot]: ###  ⚠️  No Changeset found

Latest commit: 590514eb0989886018eac497ddcb0f82e66004d6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. **If these changes should result in a version bump, you need to add a changeset.**

<details><summary>This PR includes no changesets</summary>

  When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

</details>

[Click here to learn what changesets are, and how to add one](https://github.com/changesets/changesets/blob/master/docs/adding-a-changeset.md).

[Click here if you're a maintainer who wants to add a changeset to this PR](https://github.com/remix-run/remix/new/logan/flat-route-fixes?filename=.changeset/flat-rice-stare.md&value=---%0A%22remix%22%3A%20patch%0A%22%40remix-run%2Fdev%22%3A%20patch%0A---%0A%0Afix(remix-dev)%3A%20flat%20route%20fixes%0A)



Patch:
diff --git a/.changeset/tame-swans-burn.md b/.changeset/tame-swans-burn.md
new file mode 100644
index 000000000..d63d2b113
--- /dev/null
+++ b/.changeset/tame-swans-burn.md
@@ -0,0 +1,23 @@
+---
+"remix": patch
+"@remix-run/dev": patch
+---
+
+flat route fixes and enhancements
+
+`app._index.tsx` and `app/index.tsx` are different routes. The first is an index route for the second, and will be rendered into the parent outlet. The second is the parent route itself.
+
+`index.tsx` no longer has any semantic meaning for "index routes", but rather the node module resolution convention of "index modules".
+
+`routes/app.tsx` and `routes/app/index.tsx` *are the same route*. You just moved it to a folder and made an `index.tsx` because that's how node module resolution looks for the module at `routes/app`.
+
+If you want an index route, you use `_index`
+
+| file name | route path | layout |
+|-------|-----------|--------|
+| `routes/index.tsx` | `/index` | root |
+| `routes/_index.tsx` | `/` | root |
+| `routes/app.tsx` | `/app` | root |
+| `routes/app/index.tsx` | same route as above | root |
+| `routes/app._index.tsx` | `/app` | `routes/app.tsx` or `routes/app/index.tsx` |
+| `routes/app._index/index.tsx` | same route as above | `routes/app.tsx` or `routes/app/index.tsx` |
diff --git a/packages/remix-dev/config/flat-routes.ts b/packages/remix-dev/config/flat-routes.ts
index a2e76dd73..dad8c43f7 100644
--- a/packages/remix-dev/config/flat-routes.ts
+++ b/packages/remix-dev/config/flat-routes.ts
@@ -1,7 +1,13 @@
 import path from "node:path";
 import fg from "fast-glob";
 
-import type { ConfigRoute, DefineRouteFunction, RouteManifest } from "./routes";
+import type {
+  ConfigRoute,
+  DefineRouteFunction,
+  DefineRouteOptions,
+  RouteManifest,
+} from "./routes";
+import { normalizeSlashes } from "./routes";
 import { createRouteId, defineRoutes } from "./routes";
 import {
   escapeEnd,
@@ -23,12 +29,13 @@ export function flatRoutes(
     absolute: true,
     cwd: path.join(appDirectory, "routes"),
     ignore: ignoredFilePatterns,
+    onlyFiles: true,
   });
 
   // fast-glob will return posix paths even on windows
   // convert posix to os specific paths
   let routePathsForOS = routePaths.map((routePath) => {
-    return path.join(...routePath.split(path.posix.sep));
+    return path.normalize(routePath);
   });
 
   return flatRoutesUniversal(appDirectory, routePathsForOS);
@@ -51,39 +58,47 @@ export function flatRoutesUniversal(
   prefix: string = "routes"
 ): RouteManifest {
   let routeMap = getRouteMap(appDirectory, routePaths, prefix);
-
   let uniqueRoutes = new Map<string, string>();
+  let routes = Array.from(routeMap.values());
 
   function defineNestedRoutes(
     defineRoute: DefineRouteFunction,
     parentId?: string
   ): void {
-    let childRoutes = Array.from(routeMap.values()).filter(
-      (routeInfo) => routeInfo.parentId === parentId
-    );
+    let childRoutes = routes.filter((routeInfo) => {
+      return routeInfo.parentId === parentId;
+    });
     let parentRoute = parentId ? routeMap.get(parentId) : undefined;
     let parentRoutePath = parentRoute?.path ?? "/";
     for (let childRoute of childRoutes) {
       let routePath = childRoute.path?.slice(parentRoutePath.length) ?? "";
       // remove leading slash
       routePath = routePath.replace(/^\//, "");
+
       let index = childRoute.index;
       let fullPath = childRoute.path;
 
       let uniqueRouteId = (fullPath || "") + (index ? "?index" : "");
+
       if (uniqueRouteId) {
         let conflict = uniqueRoutes.get(uniqueRouteId);
         if (conflict) {
           throw new Error(
-            `Path ${JSON.stringify(fullPath)} defined by route ${JSON.stringify(
-              childRoute.id
-            )} conflicts with route ${JSON.stringify(conflict)}`
+            `Path ${JSON.stringify(fullPath)} defined by route ` +
+              `${JSON.stringify(childRoute.id)} ` +
+              `conflicts with route ${JSON.stringify(conflict)}`
           );
         }
         uniqueRoutes.set(uniqueRouteId, childRoute.id);
       }
+
+      let childRouteOptions: DefineRouteOptions = {
+        id: path.posix.join(prefix, childRoute.id),
+        index: childRoute.index ? true : undefined,
+      };
+
       if (index) {
-        let invalidChildRoutes = Object.values(routeMap).filter(
+        let invalidChildRoutes = routes.filter(
           (routeInfo) => routeInfo.parentId === childRoute.id
         );
 
@@ -93,25 +108,20 @@ export function flatRoutesUniversal(
           );
         }
 
-        defineRoute(routePath, routeMap.get(childRoute.id!)!.file, {
-          index: true,
-        });
+        defineRoute(routePath, childRoute.file, childRouteOptions);
       } else {
-        defineRoute(routePath, routeMap.get(childRoute.id!)!.file, () => {
+        defineRoute(routePath, childRoute.file, childRouteOptions, () => {
           defineNestedRoutes(defineRoute, childRoute.id);
         });
       }
     }
   }
 
-  let routes = defineRoutes(defineNestedRoutes);
-
-  return routes;
+  return defineRoutes(defineNestedRoutes);
 }
 
 export function isIndexRoute(routeId: string) {
-  let isFlatFile = !routeId.includes(path.posix.sep);
-  return isFlatFile ? routeId.endsWith("_index") : /\/index$/.test(routeId);
+  return routeId.endsWith("_index");
 }
 
 type State =
@@ -126,12 +136,25 @@ type State =
 
 export function getRouteSegments(routeId: string) {
   let routeSegments: string[] = [];
+  let rawRouteSegments: string[] = [];
   let index = 0;
   let routeSegment = "";
   let rawRouteSegment = "";
   let state: State = "NORMAL";
-  let pushRouteSegment = (routeSegment: string) => {
-    if (!routeSegment) return;
+  let hasFolder = routeId.includes(path.posix.sep);
+
+  /**
+   * @see https://github.com/remix-run/remix/pull/5160#issuecomment-1402157424
+   */
+  if (hasFolder && (routeId.endsWith("/index") || routeId.endsWith("/route"))) {
+    let last = routeId.lastIndexOf(path.posix.sep);
+    if (last >= 0) {
+      routeId = routeId.substring(0, last);
+    }
+  }
+
+  let pushRouteSegment = (segment: string, rawSegment: string) => {
+    if (!segment) return;
 
     let notSupportedInRR = (segment: string, char: string) => {
       throw new Error(
@@ -140,18 +163,20 @@ export function getRouteSegments(routeId: string) {
       );
     };
 
-    if (rawRouteSegment.includes("*")) {
-      return notSupportedInRR(rawRouteSegment, "*");
+    if (rawSegment.includes("*")) {
+      return notSupportedInRR(rawSegment, "*");
     }
 
-    if (rawRouteSegment.includes(":")) {
-      return notSupportedInRR(rawRouteSegment, ":");
+    if (rawSegment.includes(":")) {
+      return notSupportedInRR(rawSegment, ":");
     }
 
-    if (rawRouteSegment.includes("/")) {
-      return notSupportedInRR(routeSegment, "/");
+    if (rawSegment.includes("/")) {
+      return notSupportedInRR(segment, "/");
     }
-    routeSegments.push(routeSegment);
+
+    routeSegments.push(segment);
+    rawRouteSegments.push(rawSegment);
   };
 
   while (index < routeId.length) {
@@ -161,7 +186,7 @@ export function getRouteSegments(routeId: string) {
     switch (state) {
       case "NORMAL": {
         if (isSegmentSeparator(char)) {
-          pushRouteSegment(routeSegment);
+          pushRouteSegment(routeSegment, rawRouteSegment);
           routeSegment = "";
           rawRouteSegment = "";
           state = "NORMAL";
@@ -169,10 +194,12 @@ export function getRouteSegments(routeId: string) {
         }
         if (char === escapeStart) {
           state = "ESCAPE";
+          rawRouteSegment += char;
           break;
         }
         if (char === optionalStart) {
           state = "OPTIONAL";
+          rawRouteSegment += char;
           break;
         }
         if (!routeSegment && char == paramPrefixChar) {
@@ -193,6 +220,7 @@ export function getRouteSegments(routeId: string) {
       case "ESCAPE": {
         if (char === escapeEnd) {
           state = "NORMAL";
+          rawRouteSegment += char;
           break;
         }
 
@@ -203,13 +231,14 @@ export function getRouteSegments(routeId: string) {
       case "OPTIONAL": {
         if (char === optionalEnd) {
           routeSegment += "?";
-          rawRouteSegment += "?";
+          rawRouteSegment += char;
           state = "NORMAL";
           break;
         }
 
         if (char === escapeStart) {
           state = "OPTIONAL_ESCAPE";
+          rawRouteSegment += char;
           break;
         }
 
@@ -231,6 +260,7 @@ export function getRouteSegments(routeId: string) {
       case "OPTIONAL_ESCAPE": {
         if (char === escapeEnd) {
           state = "OPTIONAL";
+          rawRouteSegment += char;
           break;
         }
 
@@ -242,8 +272,8 @@ export function getRouteSegments(routeId: string) {
   }
 
   // process remaining segment
-  pushRouteSegment(routeSegment);
-  return routeSegments;
+  pushRouteSegment(routeSegment, rawRouteSegment);
+  return [routeSegments, rawRouteSegments];
 }
 
 function findParentRouteId(
@@ -265,11 +295,13 @@ function getRouteInfo(
   filePath: string
 ): RouteInfo {
   let filePathWithoutApp = filePath.slice(appDirectory.length + 1);
-  let routeId = createRouteId(filePathWithoutApp);
+  let routeId = createFlatRouteId(filePathWithoutApp);
   let routeIdWithoutRoutes = routeId.slice(routeDirectory.length + 1);
   let index = isIndexRoute(routeIdWithoutRoutes);
-  let routeSegments = getRouteSegments(routeIdWithoutRoutes);
-  let routePath = createRoutePath(routeSegments, index);
+  let [routeSegments, rawRouteSegments] =
+    getRouteSegments(routeIdWithoutRoutes);
+
+  let routePath = createRoutePath(routeSegments, rawRouteSegments, index);
 
   return {
     id: routeIdWithoutRoutes,
@@ -281,21 +313,28 @@ function getRouteInfo(
   };
 }
 
-export function createRoutePath(routeSegments: string[], isIndex: boolean) {
+export function createRoutePath(
+  routeSegments: string[],
+  rawRouteSegments: string[],
+  isIndex: boolean
+) {
   let result = "";
 
   if (isIndex) {
     routeSegments = routeSegments.slice(0, -1);
   }
 
-  for (let segment of routeSegments) {
+  for (let index = 0; index < routeSegments.length; index++) {
+    let segment = routeSegments[index];
+    let rawSegment = rawRouteSegments[index];
+
     // skip pathless layout segments
-    if (segment.startsWith("_")) {
+    if (segment.startsWith("_") && rawSegment.startsWith("_")) {
       continue;
     }
 
     // remove trailing slash
-    if (segment.endsWith("_")) {
+    if (segment.endsWith("_") && rawSegment.endsWith("_")) {
       segment = segment.slice(0, -1);
     }
 
@@ -308,8 +347,8 @@ export function createRoutePath(routeSegments: string[], isIndex: boolean) {
 function getRouteMap(
   appDirectory: string,
   routePaths: string[],
-  prefix: string = "routes"
-) {
+  prefix: string
+): Readonly<Map<string, RouteInfo>> {
   let routeMap = new Map<string, RouteInfo>();
   let nameMap = new Map<string, RouteInfo>();
 
@@ -332,12 +371,20 @@ function getRouteMap(
   return routeMap;
 }
 
-function isRouteModuleFile(filepath: string) {
+function isRouteModuleFile(filePath: string) {
   // flat files only need correct extension
-  let isFlatFile = !filepath.includes(path.sep);
-  if (isFlatFile) {
-    return routeModuleExts.includes(path.extname(filepath));
-  }
+  let normalizedFilePath = normalizeSlashes(filePath);
+  let isFlatFile = !filePath.includes(path.posix.sep);
+  let hasExt = routeModuleExts.includes(path.extname(filePath));
+  if (isFlatFile) return hasExt;
+  let basename = normalizedFilePath.slice(0, -path.extname(filePath).length);
+  return basename.endsWith(`/route`) || basename.endsWith(`/index`);
+}
 
-  return isIndexRoute(createRouteId(filepath));
+function createFlatRouteId(filePath: string) {
+  let routeId = createRouteId(filePath);
+  if (routeId.includes(path.posix.sep) && routeId.endsWith("/index")) {
+    routeId = routeId.split(path.posix.sep).slice(0, -1).join(path.posix.sep);
+  }
+  return routeId;
 }


Test patch:
diff --git a/packages/remix-dev/__tests__/flat-routes-test.ts b/packages/remix-dev/__tests__/flat-routes-test.ts
index 5446afec4..5c55ab790 100644
--- a/packages/remix-dev/__tests__/flat-routes-test.ts
+++ b/packages/remix-dev/__tests__/flat-routes-test.ts
@@ -88,9 +88,11 @@ describe("flatRoutes", () => {
 
     for (let [input, expected] of tests) {
       it(`"${input}" -> "${expected}"`, () => {
-        let routeSegments = getRouteSegments(input);
         let isIndex = isIndexRoute(input);
-        expect(createRoutePath(routeSegments, isIndex)).toBe(expected);
+        let [routeSegments, rawRouteSegments] = getRouteSegments(input);
+        expect(createRoutePath(routeSegments, rawRouteSegments, isIndex)).toBe(
+          expected
+        );
       });
     }
 
@@ -182,7 +184,7 @@ describe("flatRoutes", () => {
         },
       ],
       [
-        "routes/_landing.tsx",
+        "routes/_landing/index.tsx",
         {
           id: "routes/_landing",
           parentId: "root",
@@ -190,12 +192,12 @@ describe("flatRoutes", () => {
         },
       ],
       [
-        "routes/_landing._index.tsx",
+        "routes/_landing._index/index.tsx",
         {
           id: "routes/_landing._index",
-          index: true,
           parentId: "routes/_landing",
           path: undefined,
+          index: true,
         },
       ],
       [
@@ -207,43 +209,26 @@ describe("flatRoutes", () => {
         },
       ],
       [
-        "routes/about.tsx",
+        "routes/_about.tsx",
         {
-          id: "routes/about",
+          id: "routes/_about",
           parentId: "root",
-          path: "about",
-        },
-      ],
-      [
-        "routes/about._index.tsx",
-        {
-          id: "routes/about._index",
-          index: true,
-          parentId: "routes/about",
           path: undefined,
         },
       ],
       [
-        "routes/about.$.tsx",
+        "routes/_about.faq.tsx",
         {
-          id: "routes/about.$",
-          parentId: "routes/about",
-          path: "*",
-        },
-      ],
-      [
-        "routes/about.faq.tsx",
-        {
-          id: "routes/about.faq",
-          parentId: "routes/about",
+          id: "routes/_about.faq",
+          parentId: "routes/_about",
           path: "faq",
         },
       ],
       [
-        "routes/about.$splat.tsx",
+        "routes/_about.$splat.tsx",
         {
-          id: "routes/about.$splat",
-          parentId: "routes/about",
+          id: "routes/_about.$splat",
+          parentId: "routes/_about",
           path: ":splat",
         },
       ],
@@ -288,6 +273,22 @@ describe("flatRoutes", () => {
           path: ":id",
         },
       ],
+      [
+        "routes/folder/route.tsx",
+        {
+          id: "routes/folder/route",
+          parentId: "root",
+          path: "folder",
+        },
+      ],
+      [
+        "routes/[route].tsx",
+        {
+          id: "routes/[route]",
+          parentId: "root",
+          path: "route",
+        },
+      ],
 
       // Opt out of parent layout
       [
@@ -326,17 +327,9 @@ describe("flatRoutes", () => {
       ],
 
       [
-        "routes/app.skipall.tsx",
-        {
-          id: "routes/app.skipall",
-          parentId: "routes/app",
-          path: "skipall",
-        },
-      ],
-      [
-        "routes/app_.skipall_/index.tsx",
+        "routes/app_.skipall_._index.tsx",
         {
-          id: "routes/app_.skipall_/index",
+          id: "routes/app_.skipall_._index",
           index: true,
           parentId: "root",
           path: "app/skipall",
@@ -345,34 +338,34 @@ describe("flatRoutes", () => {
 
       // Escaping route segments
       [
-        "routes/about.[$splat].tsx",
+        "routes/_about.[$splat].tsx",
         {
-          id: "routes/about.[$splat]",
-          parentId: "routes/about",
+          id: "routes/_about.[$splat]",
+          parentId: "routes/_about",
           path: "$splat",
         },
       ],
       [
-        "routes/about.[[].tsx",
+        "routes/_about.[[].tsx",
         {
-          id: "routes/about.[[]",
-          parentId: "routes/about",
+          id: "routes/_about.[[]",
+          parentId: "routes/_about",
           path: "[",
         },
       ],
       [
-        "routes/about.[]].tsx",
+        "routes/_about.[]].tsx",
         {
-          id: "routes/about.[]]",
-          parentId: "routes/about",
+          id: "routes/_about.[]]",
+          parentId: "routes/_about",
           path: "]",
         },
       ],
       [
-        "routes/about.[.].tsx",
+        "routes/_about.[.].tsx",
         {
-          id: "routes/about.[.]",
-          parentId: "routes/about",
+          id: "routes/_about.[.]",
+          parentId: "routes/_about",
           path: ".",
         },
       ],
@@ -477,18 +470,18 @@ describe("flatRoutes", () => {
 
       // Optional + escaped route segments
       [
-        "routes/([index]).tsx",
+        "routes/([_index]).tsx",
         {
-          id: "routes/([index])",
+          id: "routes/([_index])",
           parentId: "root",
-          path: "index?",
+          path: "_index?",
         },
       ],
       [
-        "routes/([i]ndex).([[]).([[]]).tsx",
+        "routes/(_[i]ndex).([[]).([[]]).tsx",
         {
-          id: "routes/([i]ndex).([[]).([[]])",
-          parentId: "routes/([index])",
+          id: "routes/(_[i]ndex).([[]).([[]])",
+          parentId: "routes/([_index])",
           path: "[?/[]?",
         },
       ],
@@ -584,9 +577,16 @@ describe("flatRoutes", () => {
       [
         "routes/brand/index.tsx",
         {
-          id: "routes/brand/index",
+          id: "routes/brand",
           parentId: "root",
           path: "brand",
+        },
+      ],
+      [
+        "routes/brand._index.tsx",
+        {
+          id: "routes/brand._index",
+          parentId: "routes/brand",
           index: true,
         },
       ],
@@ -600,12 +600,10 @@ describe("flatRoutes", () => {
       ],
     ];
 
-    let files: [string, Omit<ConfigRoute, "file">][] = testFiles.map(
-      ([file, route]) => {
-        let filepath = file.split("/").join(path.sep);
-        return [filepath, { ...route, file: filepath }];
-      }
-    );
+    let files: [string, ConfigRoute][] = testFiles.map(([file, route]) => {
+      let filepath = file.split("/").join(path.sep);
+      return [filepath, { ...route, file: filepath }];
+    });
 
     let routeManifest = flatRoutesUniversal(
       APP_DIR,