refactor: adjust route analysis & transformation (#2443, #2448) (CP: …

…24.4) (#2457)

* refactor(file-router): forbid same names for sibling file and directory (#2443)
* refactor(file-router): tune server routes addition in case of variable routes presented (#2448)

.gitignore

@@ -59,3 +59,6 @@ scripts/prepare/src/
 
 # nx cache
 .nx
+
+# vite cache
+.vite

packages/ts/file-router/package.json

@@ -74,7 +74,6 @@
     "chai-like": "^1.1.1",
     "deep-equal-in-any-order": "^2.0.6",
     "mocha": "^10.2.0",
-    "rimraf": "^5.0.5",
     "sinon": "^16.0.0",
     "sinon-chai": "^3.7.0",
     "type-fest": "^4.9.0"

packages/ts/file-router/src/runtime/RouterConfigurationBuilder.ts

@@ -110,19 +110,31 @@ export class RouterConfigurationBuilder {
    */
   withFallback(component: ComponentType, config?: ViewConfig): this {
     // Fallback adds two routes, so that the index (empty path) has a fallback too
-    const fallbackRoutes = [
+    const fallbackRoutes: readonly RouteObject[] = [
       { path: '*', element: createElement(component), handle: config },
       { index: true, element: createElement(component), handle: config },
     ];
 
     this.update(fallbackRoutes, (original, added, children) => {
       if (original) {
-        return children
-          ? ({
-              ...original,
-              children: [...children, ...fallbackRoutes],
-            } as RouteObject)
-          : original;
+        if (!children) {
+          return original;
+        }
+
+        const _fallback = [...fallbackRoutes];
+
+        if (children.some(({ path }) => path === '*')) {
+          _fallback.shift();
+        }
+
+        if (children.some(({ index: i, path }) => i ?? path?.includes('?'))) {
+          _fallback.pop();
+        }
+
+        return {
+          ...original,
+          children: [...children, ..._fallback],
+        } as RouteObject;
       }
 
       return added!;

packages/ts/file-router/src/vite-plugin/collectRoutesFromFS.ts

@@ -2,7 +2,8 @@ import { opendir, readFile } from 'node:fs/promises';
 import { basename, extname, relative } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import type { Logger } from 'vite';
-import { cleanUp } from './utils.js';
+import { RouteParamType } from '../shared/routeParamType.js';
+import { cleanUp, routeParamTypeMap } from './utils.js';
 
 export type RouteMeta = Readonly<{
   path: string;
@@ -75,8 +76,24 @@ export default async function collectRoutesFromFS(
       continue;
     }
 
+    const extension = extname(d.name);
+    const name = basename(d.name, extension);
+
+    if (extension !== '' && !extensions.includes(extension)) {
+      if (warningFor.includes(extension)) {
+        logger.warn(
+          `File System based router expects only JSX files in 'Frontend/views/' directory, such as '*.tsx' and '*.jsx'. The file '${d.name}' will be ignored by router, as it doesn't match this convention. Please consider storing it in another directory.`,
+        );
+      }
+      continue;
+    }
+
+    if (children.some(({ path: p }) => p === name)) {
+      throw new Error(`You cannot create a file and a directory with the same name ("${name}"). Use "@index" instead`);
+    }
+
     if (d.isDirectory()) {
-      const directoryRoutes = await collectRoutesFromFS(new URL(`${d.name}/`, dir), {
+      const directoryRoutes = await collectRoutesFromFS(new URL(`${name}/`, dir), {
         extensions,
         logger,
         parent: dir,
@@ -85,16 +102,7 @@ export default async function collectRoutesFromFS(
         const [layoutRoute] = directoryRoutes;
         children.push(layoutRoute);
       } else if (directoryRoutes.length > 0) {
-        children.push({ path: d.name, children: directoryRoutes });
-      }
-      continue;
-    }
-
-    if (!extensions.includes(extname(d.name))) {
-      if (warningFor.includes(extname(d.name))) {
-        logger.warn(
-          `File System based router expects only JSX files in 'Frontend/views/' directory, such as '*.tsx' and '*.jsx'. The file '${d.name}' will be ignored by router, as it doesn't match this convention. Please consider storing it in another directory.`,
-        );
+        children.push({ path: name, children: directoryRoutes });
       }
       continue;
     }
@@ -104,9 +112,14 @@ export default async function collectRoutesFromFS(
     if (url === undefined) {
       continue;
     }
-    const name = basename(d.name, extname(d.name));
+    const optionalParamType = routeParamTypeMap.get(RouteParamType.Optional)!;
 
-    if (name === '@layout') {
+    if (
+      (name === '@index' && children.some(({ path: p }) => p.search(optionalParamType) >= 0)) ||
+      (name.search(optionalParamType) >= 0 && children.some(({ path: p }) => p === ''))
+    ) {
+      throw new Error('You cannot create an `@index` file in a directory with optional parameters');
+    } else if (name === '@layout') {
       layout = file;
     } else if (name === '@index') {
       children.push({

packages/ts/file-router/src/vite-plugin/utils.ts

@@ -1,9 +1,9 @@
 import { RouteParamType } from '../shared/routeParamType.js';
 
-const routeParamTypeMap: ReadonlyMap<RouteParamType, RegExp> = new Map([
-  [RouteParamType.Wildcard, /\{\.{3}(.+)\}/gu],
-  [RouteParamType.Optional, /\{{2}(.+)\}{2}/gu],
-  [RouteParamType.Required, /\{(.+)\}/gu],
+export const routeParamTypeMap: ReadonlyMap<RouteParamType, RegExp> = new Map([
+  [RouteParamType.Wildcard, /\{\.\.\.(.+)\}/gu], // e.g. "{...wildcard}"
+  [RouteParamType.Optional, /\{\{(.+)\}\}/gu], // e.g. "{{param}}"
+  [RouteParamType.Required, /\{(.+)\}/gu], // e.g. "{param}"
 ]);
 
 // eslint-disable-next-line consistent-return

packages/ts/file-router/test/runtime/RouterBuilder.spec.tsx → packages/ts/file-router/test/runtime/RouterConfigurationBuilder.spec.tsx

@@ -168,24 +168,56 @@ describe('RouterBuilder', () => {
                 },
               ],
             },
+            {
+              path: '/cases',
+              children: [
+                {
+                  path: '/index',
+                  children: [
+                    {
+                      index: true,
+                      element: <div>Index</div>,
+                    },
+                  ],
+                },
+                {
+                  path: '/wildcard',
+                  children: [
+                    {
+                      path: '*',
+                      element: <div>Wildcard</div>,
+                    },
+                  ],
+                },
+                {
+                  path: '/optional',
+                  children: [
+                    {
+                      path: ':optional?',
+                      element: <div>Optional</div>,
+                    },
+                  ],
+                },
+              ],
+            },
           ],
         },
       ])
       .withFallback(Server, { title: 'Server' })
       .build();
 
-    const serverRoutes = [
-      {
-        path: '*',
-        element: <Server />,
-        handle: { title: 'Server' },
-      },
-      {
-        index: true,
-        element: <Server />,
-        handle: { title: 'Server' },
-      },
-    ];
+    const serverWildcard = {
+      path: '*',
+      element: <Server />,
+      handle: { title: 'Server' },
+    };
+    const serverIndex = {
+      index: true,
+      element: <Server />,
+      handle: { title: 'Server' },
+    };
+
+    const serverRoutes = [serverWildcard, serverIndex];
 
     expect(routes).to.be.like([
       {
@@ -215,6 +247,49 @@ describe('RouterBuilder', () => {
               ...serverRoutes,
             ],
           },
+          {
+            path: '/cases',
+            children: [
+              // If we already have an index route, all we want is to add a
+              // server wildcard
+              {
+                path: '/index',
+                children: [
+                  {
+                    index: true,
+                    element: <div>Index</div>,
+                  },
+                  serverWildcard,
+                ],
+              },
+              // If we already have a wildcard route, all we want is to add a
+              // server index.
+              {
+                path: '/wildcard',
+                children: [
+                  {
+                    path: '*',
+                    element: <div>Wildcard</div>,
+                  },
+                  serverIndex,
+                ],
+              },
+              // If we have an optional route, we just need to add a server
+              // wildcard to cover complicated cases like
+              // "/optional/something/else/deeply/nested"
+              {
+                path: '/optional',
+                children: [
+                  {
+                    path: ':optional?',
+                    element: <div>Optional</div>,
+                  },
+                  serverWildcard,
+                ],
+              },
+              ...serverRoutes,
+            ],
+          },
           ...serverRoutes,
         ],
       },