refactor(file-router): tune server routes addition in case of variabl…

…e routes presented (#2448)

* refactor: get rid of rimraf in favor of native node:fs rm

* refactor(file-router): forbid same names for directory and file

* refactor(file-router): forbid @index and {{optional}} in the same dir

* refactor(file-router): tune adding server cases in case of variable routes presented

* fix(file-router): compare correct paths in search for @index

* use regex without the global flag

* replace regex.test with string.search

* fix(file-router): check for the same file name beforehand

---------

Co-authored-by: Manuel Carrasco Moñino <manolo@vaadin.com>
Co-authored-by: taefi <taefi.soroosh@gmail.com>

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({
@@ -117,8 +130,6 @@ export default async function collectRoutesFromFS(
       throw new Error(
         'Symbol "@" is reserved for special directories and files; only "@layout" and "@index" are allowed',
       );
-    } else if (children.some(({ path: p }) => p === name)) {
-      throw new Error('You cannot create a file and a directory with the same name. Use `@index` instead.');
     } else {
       children.push({
         path: name,

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,
         ],
       },

packages/ts/file-router/test/vite-plugin/collectRoutesFromFS.spec.ts

@@ -91,7 +91,20 @@ describe('@vaadin/hilla-file-router', () => {
         ]);
 
         await expect(collectRoutesFromFS(tmp, { extensions, logger })).to.be.rejectedWith(
-          'You cannot create a file and a directory with the same name. Use `@index` instead.',
+          `You cannot create a file and a directory with the same name ("internal"). Use "@index" instead`,
+        );
+      });
+
+      it('should throw an error if there is @index and optional route in the same directory', async () => {
+        await Promise.all([
+          appendFile(
+            new URL('./{{optional}}.tsx', internalDir),
+            'export default function FailureInternalOptional() {}',
+          ),
+        ]);
+
+        await expect(collectRoutesFromFS(tmp, { extensions, logger })).to.be.rejectedWith(
+          'You cannot create an `@index` file in a directory with optional parameters',
         );
       });
     });