Repository: dai-shi/waku (ts)

Issue:
waku dev error with generated fs router types
I have a project that doesn't have a home page / root index.tsx file. I hit an error trying to run the dev server on the latest `main` commit (901594c2195de5a7d601ca6ecda674202e9a0915):

```
Error: ENOENT: no such file or directory, open '/waku-project/src/pages/index.tsx'
    at Object.openSync (node:fs:562:18)
    at readFileSync (node:fs:446:35)
    at fileExportsGetConfig (file:///waku-project/node_modules/waku/dist/lib/plugins/vite-plugin-fs-router-typegen.js:68:30)
    at generateFile (file:///waku-project/node_modules/waku/dist/lib/plugins/vite-plugin-fs-router-typegen.js:76:42)
    at updateGeneratedFile (file:///waku-project/node_modules/waku/dist/lib/plugins/vite-plugin-fs-router-typegen.js:126:51) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/waku-project/src/pages/index.tsx'
}
```

The `filePaths` given to `generateFile` in vite-plugin-fs-router-typegen.ts looks like this for my project:

```
[
  '/_layout.tsx',
  '/_layout.tsx',
  '/_layout.tsx',
  '/index.tsx',
  '/demo.tsx',
  '/index.tsx',
  '/detail.tsx',
  '/index.tsx',
  '/index.tsx',
  '/app.tsx'
]
```

I don't think that's right to not include the entry paths and only list the filenames.

~~Also, I think the `recursive` option for `fs/promises` readdir is newer than some of our targets (https://nodejs.org/api/fs.html#fspromisesreaddirpath-options).~~
 

Pull request:
fix: router typegen variable naming
Fixes #924 

Patch:
diff --git a/packages/waku/src/lib/plugins/vite-plugin-fs-router-typegen.ts b/packages/waku/src/lib/plugins/vite-plugin-fs-router-typegen.ts
index d7b5e73a..0250e536 100644
--- a/packages/waku/src/lib/plugins/vite-plugin-fs-router-typegen.ts
+++ b/packages/waku/src/lib/plugins/vite-plugin-fs-router-typegen.ts
@@ -3,53 +3,54 @@ import { readdir, writeFile } from 'node:fs/promises';
 import { existsSync, readFileSync } from 'node:fs';
 import { SRC_ENTRIES, EXTENSIONS } from '../constants.js';
 import { joinPath } from '../utils/path.js';
+import { getInputString } from '../../router/common.js';
 
 const SRC_PAGES = 'pages';
 
-const srcToName = (src: string) => {
-  const split = src
-    .split('/')
-    .map((part) => part[0]!.toUpperCase() + part.slice(1));
-
-  if (split.at(-1) === '_layout.tsx') {
-    return split.slice(0, -1).join('') + '_Layout';
-  } else if (split.at(-1) === 'index.tsx') {
-    return split.slice(0, -1).join('') + 'Index';
-  } else if (split.at(-1)?.startsWith('[...')) {
-    const fileName = split
-      .at(-1)!
-      .replace('-', '_')
-      .replace('.tsx', '')
-      .replace('[...', '')
-      .replace(']', '');
-    return (
-      split.slice(0, -1).join('') +
-      'Wild' +
-      fileName[0]!.toUpperCase() +
-      fileName.slice(1)
-    );
-  } else if (split.at(-1)?.startsWith('[')) {
-    const fileName = split
-      .at(-1)!
-      .replace('-', '_')
-      .replace('.tsx', '')
-      .replace('[', '')
-      .replace(']', '');
-    return (
-      split.slice(0, -1).join('') +
-      'Slug' +
-      fileName[0]!.toUpperCase() +
-      fileName.slice(1)
-    );
-  } else {
-    const fileName = split.at(-1)!.replace('-', '_').replace('.tsx', '');
-    return (
-      split.slice(0, -1).join('') +
-      fileName[0]!.toUpperCase() +
-      fileName.slice(1)
-    );
+// https://tc39.es/ecma262/multipage/ecmascript-language-lexical-grammar.html#sec-names-and-keywords
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#identifiers
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#reserved_words
+export function toIdentifier(input: string): string {
+  // Strip the file extension
+  let identifier = input.includes('.')
+    ? input.split('.').slice(0, -1).join('.')
+    : input;
+  // Replace any characters besides letters, numbers, underscores, and dollar signs with underscores
+  identifier = identifier.replace(/[^\p{L}\p{N}_$]/gu, '_');
+  // Ensure it starts with a letter
+  if (/^\d/.test(identifier)) {
+    identifier = '_' + identifier;
   }
-};
+  // Turn it into PascalCase
+  // Since the first letter is uppercased, it will not be a reserved word
+  return identifier
+    .split('_')
+    .map((part) => {
+      if (part[0] === undefined) return '';
+      return part[0].toUpperCase() + part.slice(1);
+    })
+    .join('');
+}
+
+export function getImportModuleNames(filePaths: string[]): {
+  [k: string]: string;
+} {
+  const moduleNameCount: { [k: string]: number } = {};
+  const moduleNames: { [k: string]: string } = {};
+  for (const filePath of filePaths) {
+    let identifier = toIdentifier(filePath);
+    moduleNameCount[identifier] = (moduleNameCount[identifier] ?? -1) + 1;
+    if (moduleNameCount[identifier]) {
+      identifier = `${identifier}_${moduleNameCount[identifier]}`;
+    }
+    try {
+      moduleNames[getInputString(filePath)] = identifier;
+    } catch (e) {
+      console.log(e);
+    }
+  }
+  return moduleNames;
+}
 
 export const fsRouterTypegenPlugin = (opts: { srcDir: string }): Plugin => {
   let entriesFilePossibilities: string[] | undefined;
@@ -91,20 +92,24 @@ export const fsRouterTypegenPlugin = (opts: { srcDir: string }): Plugin => {
       }
 
       // Recursively collect `.tsx` files in the given directory
-      const collectFiles = async (dir: string): Promise<string[]> => {
-        if (!pagesDir) return [];
-        const results: string[] = [];
-        const files = await readdir(dir, {
-          withFileTypes: true,
-          recursive: true,
-        });
-
-        for (const file of files) {
-          if (file.name.endsWith('.tsx')) {
-            results.push('/' + file.name);
+      const collectFiles = async (
+        dir: string,
+        files: string[] = [],
+      ): Promise<string[]> => {
+        // TODO revisit recursive option for readdir once more stable
+        // https://nodejs.org/docs/latest-v20.x/api/fs.html#direntparentpath
+        const entries = await readdir(dir, { withFileTypes: true });
+        for (const entry of entries) {
+          const fullPath = joinPath(dir, entry.name);
+          if (entry.isDirectory()) {
+            await collectFiles(fullPath, files);
+          } else {
+            if (entry.name.endsWith('.tsx')) {
+              files.push(pagesDir ? fullPath.slice(pagesDir.length) : fullPath);
+            }
           }
         }
-        return results;
+        return files;
       };
 
       const fileExportsGetConfig = (filePath: string) => {
@@ -119,19 +124,21 @@ export const fsRouterTypegenPlugin = (opts: { srcDir: string }): Plugin => {
 
       const generateFile = (filePaths: string[]): string => {
         const fileInfo = [];
+        const moduleNames = getImportModuleNames(filePaths);
+
         for (const filePath of filePaths) {
           // where to import the component from
-          const src = filePath.slice(1);
+          const src = getInputString(filePath);
           const hasGetConfig = fileExportsGetConfig(filePath);
 
-          if (filePath === '/_layout.tsx') {
+          if (filePath.endsWith('/_layout.tsx')) {
             fileInfo.push({
               type: 'layout',
               path: filePath.replace('_layout.tsx', ''),
               src,
               hasGetConfig,
             });
-          } else if (filePath === '/index.tsx') {
+          } else if (filePath.endsWith('/index.tsx')) {
             fileInfo.push({
               type: 'page',
               path: filePath.replace('index.tsx', ''),
@@ -152,19 +159,19 @@ export const fsRouterTypegenPlugin = (opts: { srcDir: string }): Plugin => {
 import type { PathsForPages } from 'waku/router';\n\n`;
 
         for (const file of fileInfo) {
-          const moduleName = srcToName(file.src);
+          const moduleName = moduleNames[file.src];
           result += `import ${moduleName}${file.hasGetConfig ? `, { getConfig as ${moduleName}_getConfig }` : ''} from './${SRC_PAGES}/${file.src.replace('.tsx', '')}';\n`;
         }
 
         result += `\nconst _pages = createPages(async (pagesFns) => [\n`;
 
         for (const file of fileInfo) {
-          const moduleName = srcToName(file.src);
+          const moduleName = moduleNames[file.src];
           result += `  pagesFns.${file.type === 'layout' ? 'createLayout' : 'createPage'}({ path: '${file.path}', component: ${moduleName}, ${file.hasGetConfig ? `...(await ${moduleName}_getConfig())` : `render: '${file.type === 'layout' ? 'static' : 'dynamic'}'`} }),\n`;
         }
 
         result += `]);
-  
+
   declare module 'waku/router' {
     interface RouteConfig {
       paths: PathsForPages<typeof _pages>;
diff --git "a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/\303\270n\303\251_two_three.tsx" "b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/\303\270n\303\251_two_three.tsx"
new file mode 100644
index 00000000..c046a3f8
--- /dev/null
+++ "b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/\303\270n\303\251_two_three.tsx"
@@ -0,0 +1,9 @@
+export default function Page() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'dynamic',
+  };
+};


Test patch:
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/[category]/[...tags]/index.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/[category]/[...tags]/index.tsx
new file mode 100644
index 00000000..c046a3f8
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/[category]/[...tags]/index.tsx
@@ -0,0 +1,9 @@
+export default function Page() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'dynamic',
+  };
+};
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/[category]/_layout.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/[category]/_layout.tsx
new file mode 100644
index 00000000..0b96b0dc
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/[category]/_layout.tsx
@@ -0,0 +1,9 @@
+export default function Layout() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'static',
+  };
+};
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/_layout.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/_layout.tsx
new file mode 100644
index 00000000..0b96b0dc
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/_layout.tsx
@@ -0,0 +1,9 @@
+export default function Layout() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'static',
+  };
+};
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/index.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/index.tsx
new file mode 100644
index 00000000..c046a3f8
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/index.tsx
@@ -0,0 +1,9 @@
+export default function Page() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'dynamic',
+  };
+};
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one-two-three.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one-two-three.tsx
new file mode 100644
index 00000000..c046a3f8
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one-two-three.tsx
@@ -0,0 +1,9 @@
+export default function Page() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'dynamic',
+  };
+};
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one__two_three.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one__two_three.tsx
new file mode 100644
index 00000000..c046a3f8
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one__two_three.tsx
@@ -0,0 +1,9 @@
+export default function Page() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'dynamic',
+  };
+};
diff --git a/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one_two_three.tsx b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one_two_three.tsx
new file mode 100644
index 00000000..c046a3f8
--- /dev/null
+++ b/packages/waku/tests/fixtures/plugin-fs-router-typegen/pages/one_two_three.tsx
@@ -0,0 +1,9 @@
+export default function Page() {
+  return null;
+}
+
+export const getConfig = async () => {
+  return {
+    render: 'dynamic',
+  };
+};
diff --git a/packages/waku/tests/vite-plugin-fs-router-typegen.test.ts b/packages/waku/tests/vite-plugin-fs-router-typegen.test.ts
new file mode 100644
index 00000000..9d02f7c5
--- /dev/null
+++ b/packages/waku/tests/vite-plugin-fs-router-typegen.test.ts
@@ -0,0 +1,98 @@
+import { describe, expect, test, vi } from 'vitest';
+import {
+  fsRouterTypegenPlugin,
+  getImportModuleNames,
+  toIdentifier,
+} from '../src/lib/plugins/vite-plugin-fs-router-typegen.js';
+import { fileURLToPath } from 'node:url';
+import { FSWatcher, ViteDevServer } from 'vite';
+import { writeFile } from 'node:fs/promises';
+
+const root = fileURLToPath(new URL('./fixtures', import.meta.url));
+
+vi.mock('prettier', () => {
+  return { format: (x: string) => x, resolveConfig: () => ({}) };
+});
+vi.mock('node:fs/promises', async (importOriginal) => {
+  const mod = await importOriginal();
+  return {
+    // https://vitest.dev/api/vi.html#vi-mock
+    // @ts-expect-error - docs say this should be inferred...
+    ...mod,
+    writeFile: vi.fn(),
+  };
+});
+
+async function runTest(
+  root: string,
+  expectedEntriesGen: string,
+  srcDir = 'plugin-fs-router-typegen',
+) {
+  const plugin = fsRouterTypegenPlugin({
+    srcDir,
+  });
+  expect(plugin.configureServer).toBeDefined();
+  expect(typeof plugin.configureServer).toBe('function');
+  expect(plugin.configResolved).toBeDefined();
+  expect(typeof plugin.configResolved).toBe('function');
+  if (
+    typeof plugin.configureServer !== 'function' ||
+    typeof plugin.configResolved !== 'function'
+  ) {
+    return;
+  }
+  // @ts-expect-error - we're not passing the full Vite config
+  await plugin.configResolved?.({ root });
+  await plugin.configureServer?.({
+    watcher: { add: () => {}, on: () => {} } as unknown as FSWatcher,
+  } as ViteDevServer);
+  await vi.waitFor(async () => {
+    if (vi.mocked(writeFile).mock.lastCall === undefined) {
+      throw new Error('writeFile not called');
+    }
+  });
+  expect(vi.mocked(writeFile).mock.lastCall?.[1]).toContain(expectedEntriesGen);
+}
+
+describe('vite-plugin-fs-router-typegen', () => {
+  test('generates valid module names for fs entries', async () => {
+    expect(toIdentifier('/_layout.tsx')).toBe('Layout');
+    expect(toIdentifier('/[category]/[...tags]/index.tsx')).toBe(
+      'CategoryTagsIndex',
+    );
+  });
+
+  test('allows unicode characters in module names', async () => {
+    expect(toIdentifier('/øné_two_three.tsx')).toBe('ØnéTwoThree');
+  });
+
+  test('handles collisions of fs entry module names', async () => {
+    expect(
+      getImportModuleNames([
+        '/one-two-three.tsx',
+        '/one/two/three.tsx',
+        '/one_two_three.tsx',
+        '/one__two_three.tsx',
+      ]),
+    ).toEqual({
+      'one-two-three.tsx': 'OneTwoThree',
+      'one/two/three.tsx': 'OneTwoThree_1',
+      'one_two_three.tsx': 'OneTwoThree_2',
+      'one__two_three.tsx': 'OneTwoThree_3',
+    });
+  });
+
+  test('creates the expected imports the generated entries file', async () => {
+    await runTest(
+      root,
+      `import CategoryTagsIndex, { getConfig as CategoryTagsIndex_getConfig } from './pages/[category]/[...tags]/index';
+import CategoryLayout, { getConfig as CategoryLayout_getConfig } from './pages/[category]/_layout';
+import Layout, { getConfig as Layout_getConfig } from './pages/_layout';
+import Index, { getConfig as Index_getConfig } from './pages/index';
+import OneTwoThree, { getConfig as OneTwoThree_getConfig } from './pages/one-two-three';
+import OneTwoThree_1, { getConfig as OneTwoThree_1_getConfig } from './pages/one__two_three';
+import OneTwoThree_2, { getConfig as OneTwoThree_2_getConfig } from './pages/one_two_three';
+import ØnéTwoThree, { getConfig as ØnéTwoThree_getConfig } from './pages/øné_two_three';`,
+    );
+  });
+});