From bad5e80b698b804f60440fe4298603b67db085d2 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 22 Oct 2026 12:43:49 +0800 Subject: [PATCH] fix(pages): report page route clashes and match encoded page paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - scanPages throws one error listing every route or page id that more than one page file resolves to (about.tsx + about/index.tsx, .tsx/.vue twins, the same page in two route groups, index.tsx + index/index.tsx). These had no stable winner: the server and the client could each pick a different file. Dynamic pages that match the same URLs ([id] and [slug]) only warn, once, since the first one always wins today. - The checks are skipped when another framework owns pages/ (Nuxt nests users.vue and users/index.vue). - A page in a folder named layout (layout/index.tsx) no longer loads the layout beside it: only then is that layout keyed "_layout" in the manifest. Nothing changes for apps without the clash. - Client navigation decodes the path the way the server (Hono) does before matching, so pages such as café.tsx or "a b.tsx" match on the client. The generated matchRoute and matchAdminRoute take that path. - void dev logs a page scan error and keeps the last good route table instead of exiting, and leaves the log to the adapter pages plugin when it reports the same error. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/guide/pages-routing/overview.md | 2 + packages/react/src/runtime/pages-client.ts | 3 +- packages/void/src/application-routing.ts | 2 + packages/void/src/pages/manifest.ts | 30 ++++-- packages/void/src/pages/navigation-engine.ts | 9 +- packages/void/src/pages/scan.ts | 94 +++++++++++++++- packages/void/src/pages/url.ts | 47 ++++++++ packages/void/src/plugin-routing.ts | 21 +++- packages/void/src/router/scan.ts | 7 +- .../void/test/unit/pages-manifest.test.ts | 101 ++++++++++++++++-- packages/void/test/unit/pages-scan.test.ts | 78 ++++++++++++++ .../void/test/unit/plugin-routing.test.ts | 48 ++++++++- packages/void/test/unit/scan.test.ts | 19 ++++ .../admin/react/AdminNavigationFallback.tsx | 3 +- .../packages/api/src/admin/react/pages.ts | 4 +- 15 files changed, 442 insertions(+), 26 deletions(-) diff --git a/docs/guide/pages-routing/overview.md b/docs/guide/pages-routing/overview.md index 01fab3c9c..174b72fc6 100644 --- a/docs/guide/pages-routing/overview.md +++ b/docs/guide/pages-routing/overview.md @@ -94,6 +94,8 @@ Pages can be flat files such as `about.vue` or directory-based routes such as `a +Keep one file per route. Void reports an error when two page files resolve to the same path, such as `about.vue` and `about/index.vue`, or the same page in two route groups. + Each page can have a companion `.server.ts` file that runs exclusively on the server. It can export: - A [**loader**](./loaders), which runs on `GET` and returns the data that becomes the page component's props diff --git a/packages/react/src/runtime/pages-client.ts b/packages/react/src/runtime/pages-client.ts index 97688b3a5..3b050f3a4 100644 --- a/packages/react/src/runtime/pages-client.ts +++ b/packages/react/src/runtime/pages-client.ts @@ -48,7 +48,8 @@ export type ReactPagesConfig = Readonly<{ components: Record Promise>; layoutTree: Record>; routeMeta: Record; - matchRoute(url: string): string | null; + /** Page id for a decoded pathname. */ + matchRoute(path: string): string | null; staticPageData: boolean; staticPageDataFastPath: boolean; viewTransitions: boolean; diff --git a/packages/void/src/application-routing.ts b/packages/void/src/application-routing.ts index 48d7c6fb7..fd21d1660 100644 --- a/packages/void/src/application-routing.ts +++ b/packages/void/src/application-routing.ts @@ -10,6 +10,8 @@ export async function completeApplicationAnalysis( const scan = await scanRoutes(analysis.root, { output: analysis.config.output, paths: analysis.paths, + // A framework app's pages/ follows that framework's rules, not Void Pages. + checkPageRoutes: !analysis.framework, inventory: { routes: analysis.routes, middleware: analysis.middleware, diff --git a/packages/void/src/pages/manifest.ts b/packages/void/src/pages/manifest.ts index e6485cb33..b0757b123 100644 --- a/packages/void/src/pages/manifest.ts +++ b/packages/void/src/pages/manifest.ts @@ -8,11 +8,24 @@ import type { PageScanResult } from './types.ts'; * Client entries load this eagerly, so it grows with every page. Each page id * is written once, identical layout chains are shared, and static routes are * matched by exact lookup instead of a RegExp per page. The emitted bindings - * (`components`, `layoutTree`, `routeMeta`, `matchRoute`) keep their shape. + * (`components`, `layoutTree`, `routeMeta`, `matchRoute`) keep their shape; + * `matchRoute` takes a decoded pathname (see `decodePagePath`). */ export function generateComponentManifest(scan: PageScanResult, pagesDir: string): string { const load = (filePath: string) => `() => import(${JSON.stringify(`${pagesDir}/${filePath}`)})`; + // A page in a folder named "layout" (layout/index.tsx) has the id of the + // layout file beside it (layout.tsx). No page path has a segment starting + // with "_", so that layout is keyed "_layout" instead. + const pageIds = new Set(scan.pages.map((page) => page.componentId)); + const layoutKey = (id: string): string => { + if (!pageIds.has(id)) { + return id; + } + const slash = id.lastIndexOf('/'); + return id.slice(0, slash + 1) + '_' + id.slice(slash + 1); + }; + const chains: Array = []; const chainIndexes = new Map(); const staticRows: Array = []; @@ -21,7 +34,9 @@ export function generateComponentManifest(scan: PageScanResult, pagesDir: string // can be looked up exactly without changing which route matches first. let inStaticPrefix = true; for (const page of scan.pages) { - const chain = JSON.stringify(resolveLayoutIds(page, scan.layouts, scan.namedLayouts)); + const chain = JSON.stringify( + resolveLayoutIds(page, scan.layouts, scan.namedLayouts).map(layoutKey), + ); let chainIndex = chain === '[]' ? -1 : chainIndexes.get(chain); if (chainIndex === undefined) { chainIndex = chains.push(chain) - 1; @@ -60,9 +75,10 @@ for (const [pattern, id, load, chain] of __voidDynamicRoutes) { if (chain >= 0) layoutTree[id] = __voidLayoutChains[chain]; routes.push({ pattern, id }); }`); - // Layouts are set after pages, so a layout keeps an id it shares with a page. for (const layout of [...scan.layouts, ...scan.namedLayouts]) { - lines.push(`components[${JSON.stringify(toLayoutId(layout))}] = ${load(layout.filePath)};`); + lines.push( + `components[${JSON.stringify(layoutKey(toLayoutId(layout)))}] = ${load(layout.filePath)};`, + ); } // Route metadata (sparse — only island pages) @@ -74,8 +90,7 @@ for (const [pattern, id, load, chain] of __voidDynamicRoutes) { } lines.push('};'); - lines.push(`function matchRoute(url) { - const path = new URL(url, "http://x").pathname; + lines.push(`function matchRoute(path) { const id = __voidStaticRouteIds.get(path); if (id !== undefined) return id; for (const r of routes) { @@ -106,7 +121,8 @@ function isStaticPattern(pattern: string): boolean { return pattern.split('/').every((segment) => captureGroup(segment) === undefined); } -function patternToRegex(pattern: string): string { +/** The RegExp literal a page pattern matches with, the same on the client and the server. */ +export function patternToRegex(pattern: string): string { const regexSource = pattern .split('/') .map((segment) => { diff --git a/packages/void/src/pages/navigation-engine.ts b/packages/void/src/pages/navigation-engine.ts index 62b869ad2..217981c09 100644 --- a/packages/void/src/pages/navigation-engine.ts +++ b/packages/void/src/pages/navigation-engine.ts @@ -8,7 +8,7 @@ import { PrefetchCache, parseCacheFor } from './prefetch.ts'; import type { CacheForInput, CacheForResult } from './prefetch.ts'; import type { PageObject } from './protocol.ts'; import { createScrollRestoration } from './scroll-restoration.ts'; -import { isSameOrigin } from './url.ts'; +import { decodePagePath, isSameOrigin } from './url.ts'; /** * The server leaves a named action's key out of the page URL. When the page URL @@ -77,7 +77,8 @@ export type PageNavigationEngineConfig = { components: Record Promise>; layoutTree: Record>; routeMeta: Record; - matchRoute(url: string): string | null; + /** Page id for a decoded pathname (see `decodePagePath`). */ + matchRoute(path: string): string | null; staticPageData?: boolean; staticPageDataFastPath?: boolean; viewTransitions: boolean; @@ -239,7 +240,6 @@ export async function createPageNavigationEngine( components, layoutTree, routeMeta, - matchRoute, staticPageData: STATIC_PAGE_DATA_ENABLED = false, staticPageDataFastPath: STATIC_PAGE_DATA_FAST_PATH = STATIC_PAGE_DATA_ENABLED, prefetchHoverDelay: PREFETCH_HOVER_DELAY, @@ -247,6 +247,9 @@ export async function createPageNavigationEngine( cacheVisitedPageData: CACHE_VISITED_PAGE_DATA = false, } = config; + const matchRoute = (url: string) => + config.matchRoute(decodePagePath(new URL(url, 'http://x').pathname)); + const _prefetchCache = new PrefetchCache(); let _nextNavigationId = 0; let _pendingResource: PageNavigationResource | null = null; diff --git a/packages/void/src/pages/scan.ts b/packages/void/src/pages/scan.ts index e8f53635c..53fd11823 100644 --- a/packages/void/src/pages/scan.ts +++ b/packages/void/src/pages/scan.ts @@ -19,6 +19,7 @@ import { import { staticExportInitializers, staticLiteral } from '../shared/static-exports.ts'; import { scriptBlocks } from './island-imports.ts'; import { resolveNamedLayout } from './layout-resolution.ts'; +import { patternToRegex } from './manifest.ts'; import type { ExplicitBoolean, PageDefinition, @@ -37,6 +38,8 @@ export const MAX_REVALIDATE = 31536000; // 1 year in seconds /** Layout paths already reported; a dev server rescans pages on every file change. */ const warnedLayoutPaths = new Set(); +/** Unreachable pages already reported, for the same reason. */ +const warnedShadowedPages = new Set(); type PageRenderingSource = { componentPath: string; @@ -49,7 +52,13 @@ type PageRenderingSource = { hasGetPrerenderPaths?: boolean; }; -type PageScanOptions = { output?: 'server' | 'static'; paths?: ProjectPaths; pagesDir?: string }; +type PageScanOptions = { + output?: 'server' | 'static'; + paths?: ProjectPaths; + pagesDir?: string; + /** Report pages that share a route. Off when another framework owns pages/ (Nuxt nests users.vue and users/index.vue). */ + checkRoutes?: boolean; +}; type ResolvedPageRendering = { ssrMode: SsrMode; @@ -363,6 +372,11 @@ export async function scanPages(root: string, options?: PageScanOptions): Promis return a.pattern.localeCompare(b.pattern); }); + if (options?.checkRoutes !== false) { + assertOnePagePerRoute(pages); + warnShadowedPages(pagesDir, pages); + } + // Validate: throw when regular pages are under island layouts for (const page of pages) { if (page.island) { @@ -405,6 +419,84 @@ export async function scanPages(root: string, options?: PageScanOptions): Promis return { pages, layouts, namedLayouts }; } +function compareText(left: string, right: string): number { + return left < right ? -1 : left > right ? 1 : 0; +} + +/** Group sorted pages by `key`, keeping only groups with more than one page. */ +function pagesSharing( + pages: Array, + key: (page: PageDefinition) => string, +): Array> { + const groups = new Map>(); + for (const page of pages) { + const group = groups.get(key(page)); + if (group) { + group.push(page); + } else { + groups.set(key(page), [page]); + } + } + return [...groups.values()].filter((group) => group.length > 1); +} + +/** + * Two files for one path have no stable winner: the server and the client can + * each pick a different one, and the order can change between runs. + */ +function assertOnePagePerRoute(pages: Array): void { + assertNoneShare( + pages, + (page) => page.pattern, + 'pages: Multiple page files resolve to the same route:', + 'Keep one page file per route and rename or remove the others.', + ); + // index.tsx and index/index.tsx have different routes but the same page id. + assertNoneShare( + pages, + (page) => page.componentId, + 'pages: Multiple page files resolve to the same page id:', + 'Rename or remove one of them.', + ); +} + +function assertNoneShare( + pages: Array, + key: (page: PageDefinition) => string, + title: string, + hint: string, +): void { + const clashes = pagesSharing(pages, key) + .map((group) => ({ + key: key(group[0]), + files: group.map((page) => page.componentPath).sort(compareText), + })) + .sort((left, right) => compareText(left.key, right.key)) + .map( + (clash) => + ` ${JSON.stringify(clash.key)}: ${clash.files.map((file) => JSON.stringify(file)).join(', ')}`, + ); + if (clashes.length > 0) { + throw new Error([title, ...clashes, hint].join('\n')); + } +} + +/** Dynamic routes that match the same URLs, such as [id] and [slug]: only the first is ever used. */ +function warnShadowedPages(pagesDir: string, pages: Array): void { + for (const [used, ...shadowed] of pagesSharing(pages, (page) => patternToRegex(page.pattern))) { + for (const page of shadowed) { + const key = `${pagesDir}\0${used.componentPath}\0${page.componentPath}`; + if (warnedShadowedPages.has(key)) { + continue; + } + warnedShadowedPages.add(key); + voidWarn( + `pages: Page '${page.componentPath}' (${page.pattern}) is never used: '${used.componentPath}' (${used.pattern}) matches the same URLs first. Remove one of them.`, + ); + } + } +} + // Same leading-script rule as @void/md's extractScript: a markdown page may open // with a