-
-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: router typegen variable naming #925
Changes from all commits
00d35d4
e7dfef7
9bb2d88
ba1ef41
784686e
f008407
42705d3
c410344
7f7bd9f
862e240
a25053e
16690bb
676ba0e
6461aaa
63311ff
034f8dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should filePath be renamed? or is this still right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. filePath seems right. |
||
|
@@ -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')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this needed? same comment for both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's needed for nested index and layout files. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh the filepaths are all relative to pages dir now, that makes sense |
||
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>; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Page() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'dynamic', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Layout() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'static', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Layout() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'static', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Page() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'dynamic', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Page() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'dynamic', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Page() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'dynamic', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Page() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'dynamic', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export default function Page() { | ||
return null; | ||
} | ||
|
||
export const getConfig = async () => { | ||
return { | ||
render: 'dynamic', | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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';`, | ||
); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dai-shi this is going back to the manual recursive collection of files since the recursive flag seems to not be supported on node 18 if I followed correctly.
@rmarscher can you add a todo to switch this back whenever we have support for
recursive: true
as stable?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.