Skip to content

Commit cedcd98

Browse files
Brooooooklynclaude
andauthored
fix(pages): report page route clashes and match encoded page paths (#545)
Follow-up to #534. Fixes page routing bugs that were already on main. ## What was broken | Case | Before | |---|---| | `about.tsx` + `about/index.tsx`, `x.tsx` + `x.vue`, `(a)/team.tsx` + `(b)/team.tsx` | Two files for one route, no warning. The server and the client could each pick a different file (one file's loader fed the other file's component). Route groups changed winner between runs. With two loaders, dev failed with `Identifier '_pload_about' has already been declared`. | | `index.tsx` + `index/index.tsx` | Different routes, same page id `index`, so one route rendered the other page. | | `layout/index.tsx` next to `layout.tsx` | The page had the layout's id, so `/layout` rendered the layout as the page. | | `café.tsx`, `a b.tsx` | Never matched during client navigation: the client compared the encoded `/caf%C3%A9` with the raw pattern. | | A clashing page added while `void dev` runs | The dev server process exited. | ## What changes ``` scanPages same route / same page id → one error, every clash listed, sorted [id] + [slug] (same URLs) → one warning; the first page always wins today framework app (Nuxt …) → no check: users.vue + users/index.vue is Nuxt nesting manifest layout id == page id → that layout is keyed "_layout" (only then) client navigation URL → pathname → decodePagePath() → matchRoute(path) decodePagePath copies Hono's getPath decoding (no regex) void dev scan error → logged once, last good route table kept ``` Example error: ``` pages: Multiple page files resolve to the same route: "/about": "about.tsx", "about/index.tsx" "/team": "(a)/team.tsx", "(b)/team.tsx" Keep one page file per route and rename or remove the others. ``` Notes: - Apps without a clash get the same ids and manifest rows. The generated `matchRoute` now takes a decoded pathname. The engine and `matchAdminRoute` follow that. - The dev guard keeps the last good route table for any scan error, not only page clashes. - No repo playground has a clash: all 17 pages dirs were scanned. ## Tests - `pages-scan.test.ts`: the route clash error (exact text), the page id clash, the warning printed once across rescans, and no false positive for `/users/[id]/edit` next to `/users/[userId]/posts` or for `layout.tsx` + `layout/index.tsx`. - `scan.test.ts`: a Void app with `users.vue` + `users/index.vue` throws, and a Nuxt app does not. - `pages-manifest.test.ts`: the layout folder case. Encoded paths are checked against a real Hono app for 19 URLs (`%C3%A9`, `%c3%a9`, `%20`, `%25`, `%2525`, `%2F`, `%zz`, over-encoded ASCII, query/hash), with and without a root `[slug]`. - `plugin-routing.test.ts`: moving `about.tsx` to `about/index.tsx` in dev logs once, keeps the old table, and recovers on unlink. Nothing is logged when an adapter pages plugin is present. - Each new test fails on main. Unit suites (7,564), the browser tests for all four adapters and the smart runner's affected integration files pass locally. ## Not in this PR (already on main) - `routes/` has the same silent first-file-wins clash. `.dev`/`.prod` pairs are intended there, so it needs its own rule. - `a-b.tsx` and `a_b.tsx`, each with a `.server.ts`, both become `_pload_a_b` (dev SyntaxError). - ISR page TTLs compare the encoded pathname with the raw pattern (`@void/edge` `matchSegmentPattern`). - An unnamed catch-all `[...].tsx` matches the bare `/files` on the server but not on the client. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 9da69ac commit cedcd98

15 files changed

Lines changed: 442 additions & 26 deletions

File tree

‎docs/guide/pages-routing/overview.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ Pages can be flat files such as `about.vue` or directory-based routes such as `a
9494

9595
<PagesFileTree />
9696

97+
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.
98+
9799
Each page can have a companion `.server.ts` file that runs exclusively on the server. It can export:
98100

99101
- A [**loader**](./loaders), which runs on `GET` and returns the data that becomes the page component's props

‎packages/react/src/runtime/pages-client.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ export type ReactPagesConfig = Readonly<{
4848
components: Record<string, () => Promise<ComponentModule>>;
4949
layoutTree: Record<string, Array<string>>;
5050
routeMeta: Record<string, { island?: boolean }>;
51-
matchRoute(url: string): string | null;
51+
/** Page id for a decoded pathname. */
52+
matchRoute(path: string): string | null;
5253
staticPageData: boolean;
5354
staticPageDataFastPath: boolean;
5455
viewTransitions: boolean;

‎packages/void/src/application-routing.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ export async function completeApplicationAnalysis(
1010
const scan = await scanRoutes(analysis.root, {
1111
output: analysis.config.output,
1212
paths: analysis.paths,
13+
// A framework app's pages/ follows that framework's rules, not Void Pages.
14+
checkPageRoutes: !analysis.framework,
1315
inventory: {
1416
routes: analysis.routes,
1517
middleware: analysis.middleware,

‎packages/void/src/pages/manifest.ts‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,24 @@ import type { PageScanResult } from './types.ts';
88
* Client entries load this eagerly, so it grows with every page. Each page id
99
* is written once, identical layout chains are shared, and static routes are
1010
* matched by exact lookup instead of a RegExp per page. The emitted bindings
11-
* (`components`, `layoutTree`, `routeMeta`, `matchRoute`) keep their shape.
11+
* (`components`, `layoutTree`, `routeMeta`, `matchRoute`) keep their shape;
12+
* `matchRoute` takes a decoded pathname (see `decodePagePath`).
1213
*/
1314
export function generateComponentManifest(scan: PageScanResult, pagesDir: string): string {
1415
const load = (filePath: string) => `() => import(${JSON.stringify(`${pagesDir}/${filePath}`)})`;
1516

17+
// A page in a folder named "layout" (layout/index.tsx) has the id of the
18+
// layout file beside it (layout.tsx). No page path has a segment starting
19+
// with "_", so that layout is keyed "_layout" instead.
20+
const pageIds = new Set(scan.pages.map((page) => page.componentId));
21+
const layoutKey = (id: string): string => {
22+
if (!pageIds.has(id)) {
23+
return id;
24+
}
25+
const slash = id.lastIndexOf('/');
26+
return id.slice(0, slash + 1) + '_' + id.slice(slash + 1);
27+
};
28+
1629
const chains: Array<string> = [];
1730
const chainIndexes = new Map<string, number>();
1831
const staticRows: Array<string> = [];
@@ -21,7 +34,9 @@ export function generateComponentManifest(scan: PageScanResult, pagesDir: string
2134
// can be looked up exactly without changing which route matches first.
2235
let inStaticPrefix = true;
2336
for (const page of scan.pages) {
24-
const chain = JSON.stringify(resolveLayoutIds(page, scan.layouts, scan.namedLayouts));
37+
const chain = JSON.stringify(
38+
resolveLayoutIds(page, scan.layouts, scan.namedLayouts).map(layoutKey),
39+
);
2540
let chainIndex = chain === '[]' ? -1 : chainIndexes.get(chain);
2641
if (chainIndex === undefined) {
2742
chainIndex = chains.push(chain) - 1;
@@ -60,9 +75,10 @@ for (const [pattern, id, load, chain] of __voidDynamicRoutes) {
6075
if (chain >= 0) layoutTree[id] = __voidLayoutChains[chain];
6176
routes.push({ pattern, id });
6277
}`);
63-
// Layouts are set after pages, so a layout keeps an id it shares with a page.
6478
for (const layout of [...scan.layouts, ...scan.namedLayouts]) {
65-
lines.push(`components[${JSON.stringify(toLayoutId(layout))}] = ${load(layout.filePath)};`);
79+
lines.push(
80+
`components[${JSON.stringify(layoutKey(toLayoutId(layout)))}] = ${load(layout.filePath)};`,
81+
);
6682
}
6783

6884
// Route metadata (sparse — only island pages)
@@ -74,8 +90,7 @@ for (const [pattern, id, load, chain] of __voidDynamicRoutes) {
7490
}
7591
lines.push('};');
7692

77-
lines.push(`function matchRoute(url) {
78-
const path = new URL(url, "http://x").pathname;
93+
lines.push(`function matchRoute(path) {
7994
const id = __voidStaticRouteIds.get(path);
8095
if (id !== undefined) return id;
8196
for (const r of routes) {
@@ -106,7 +121,8 @@ function isStaticPattern(pattern: string): boolean {
106121
return pattern.split('/').every((segment) => captureGroup(segment) === undefined);
107122
}
108123

109-
function patternToRegex(pattern: string): string {
124+
/** The RegExp literal a page pattern matches with, the same on the client and the server. */
125+
export function patternToRegex(pattern: string): string {
110126
const regexSource = pattern
111127
.split('/')
112128
.map((segment) => {

‎packages/void/src/pages/navigation-engine.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { PrefetchCache, parseCacheFor } from './prefetch.ts';
88
import type { CacheForInput, CacheForResult } from './prefetch.ts';
99
import type { PageObject } from './protocol.ts';
1010
import { createScrollRestoration } from './scroll-restoration.ts';
11-
import { isSameOrigin } from './url.ts';
11+
import { decodePagePath, isSameOrigin } from './url.ts';
1212

1313
/**
1414
* The server leaves a named action's key out of the page URL. When the page URL
@@ -77,7 +77,8 @@ export type PageNavigationEngineConfig = {
7777
components: Record<string, () => Promise<PageComponentModule>>;
7878
layoutTree: Record<string, Array<string>>;
7979
routeMeta: Record<string, { island?: boolean }>;
80-
matchRoute(url: string): string | null;
80+
/** Page id for a decoded pathname (see `decodePagePath`). */
81+
matchRoute(path: string): string | null;
8182
staticPageData?: boolean;
8283
staticPageDataFastPath?: boolean;
8384
viewTransitions: boolean;
@@ -239,14 +240,16 @@ export async function createPageNavigationEngine(
239240
components,
240241
layoutTree,
241242
routeMeta,
242-
matchRoute,
243243
staticPageData: STATIC_PAGE_DATA_ENABLED = false,
244244
staticPageDataFastPath: STATIC_PAGE_DATA_FAST_PATH = STATIC_PAGE_DATA_ENABLED,
245245
prefetchHoverDelay: PREFETCH_HOVER_DELAY,
246246
prefetchDefaultCacheFor: PREFETCH_DEFAULT_CACHE_FOR,
247247
cacheVisitedPageData: CACHE_VISITED_PAGE_DATA = false,
248248
} = config;
249249

250+
const matchRoute = (url: string) =>
251+
config.matchRoute(decodePagePath(new URL(url, 'http://x').pathname));
252+
250253
const _prefetchCache = new PrefetchCache();
251254
let _nextNavigationId = 0;
252255
let _pendingResource: PageNavigationResource | null = null;

‎packages/void/src/pages/scan.ts‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import { staticExportInitializers, staticLiteral } from '../shared/static-exports.ts';
2020
import { scriptBlocks } from './island-imports.ts';
2121
import { resolveNamedLayout } from './layout-resolution.ts';
22+
import { patternToRegex } from './manifest.ts';
2223
import type {
2324
ExplicitBoolean,
2425
PageDefinition,
@@ -37,6 +38,8 @@ export const MAX_REVALIDATE = 31536000; // 1 year in seconds
3738

3839
/** Layout paths already reported; a dev server rescans pages on every file change. */
3940
const warnedLayoutPaths = new Set<string>();
41+
/** Unreachable pages already reported, for the same reason. */
42+
const warnedShadowedPages = new Set<string>();
4043

4144
type PageRenderingSource = {
4245
componentPath: string;
@@ -49,7 +52,13 @@ type PageRenderingSource = {
4952
hasGetPrerenderPaths?: boolean;
5053
};
5154

52-
type PageScanOptions = { output?: 'server' | 'static'; paths?: ProjectPaths; pagesDir?: string };
55+
type PageScanOptions = {
56+
output?: 'server' | 'static';
57+
paths?: ProjectPaths;
58+
pagesDir?: string;
59+
/** Report pages that share a route. Off when another framework owns pages/ (Nuxt nests users.vue and users/index.vue). */
60+
checkRoutes?: boolean;
61+
};
5362

5463
type ResolvedPageRendering = {
5564
ssrMode: SsrMode;
@@ -363,6 +372,11 @@ export async function scanPages(root: string, options?: PageScanOptions): Promis
363372
return a.pattern.localeCompare(b.pattern);
364373
});
365374

375+
if (options?.checkRoutes !== false) {
376+
assertOnePagePerRoute(pages);
377+
warnShadowedPages(pagesDir, pages);
378+
}
379+
366380
// Validate: throw when regular pages are under island layouts
367381
for (const page of pages) {
368382
if (page.island) {
@@ -405,6 +419,84 @@ export async function scanPages(root: string, options?: PageScanOptions): Promis
405419
return { pages, layouts, namedLayouts };
406420
}
407421

422+
function compareText(left: string, right: string): number {
423+
return left < right ? -1 : left > right ? 1 : 0;
424+
}
425+
426+
/** Group sorted pages by `key`, keeping only groups with more than one page. */
427+
function pagesSharing(
428+
pages: Array<PageDefinition>,
429+
key: (page: PageDefinition) => string,
430+
): Array<Array<PageDefinition>> {
431+
const groups = new Map<string, Array<PageDefinition>>();
432+
for (const page of pages) {
433+
const group = groups.get(key(page));
434+
if (group) {
435+
group.push(page);
436+
} else {
437+
groups.set(key(page), [page]);
438+
}
439+
}
440+
return [...groups.values()].filter((group) => group.length > 1);
441+
}
442+
443+
/**
444+
* Two files for one path have no stable winner: the server and the client can
445+
* each pick a different one, and the order can change between runs.
446+
*/
447+
function assertOnePagePerRoute(pages: Array<PageDefinition>): void {
448+
assertNoneShare(
449+
pages,
450+
(page) => page.pattern,
451+
'pages: Multiple page files resolve to the same route:',
452+
'Keep one page file per route and rename or remove the others.',
453+
);
454+
// index.tsx and index/index.tsx have different routes but the same page id.
455+
assertNoneShare(
456+
pages,
457+
(page) => page.componentId,
458+
'pages: Multiple page files resolve to the same page id:',
459+
'Rename or remove one of them.',
460+
);
461+
}
462+
463+
function assertNoneShare(
464+
pages: Array<PageDefinition>,
465+
key: (page: PageDefinition) => string,
466+
title: string,
467+
hint: string,
468+
): void {
469+
const clashes = pagesSharing(pages, key)
470+
.map((group) => ({
471+
key: key(group[0]),
472+
files: group.map((page) => page.componentPath).sort(compareText),
473+
}))
474+
.sort((left, right) => compareText(left.key, right.key))
475+
.map(
476+
(clash) =>
477+
` ${JSON.stringify(clash.key)}: ${clash.files.map((file) => JSON.stringify(file)).join(', ')}`,
478+
);
479+
if (clashes.length > 0) {
480+
throw new Error([title, ...clashes, hint].join('\n'));
481+
}
482+
}
483+
484+
/** Dynamic routes that match the same URLs, such as [id] and [slug]: only the first is ever used. */
485+
function warnShadowedPages(pagesDir: string, pages: Array<PageDefinition>): void {
486+
for (const [used, ...shadowed] of pagesSharing(pages, (page) => patternToRegex(page.pattern))) {
487+
for (const page of shadowed) {
488+
const key = `${pagesDir}\0${used.componentPath}\0${page.componentPath}`;
489+
if (warnedShadowedPages.has(key)) {
490+
continue;
491+
}
492+
warnedShadowedPages.add(key);
493+
voidWarn(
494+
`pages: Page '${page.componentPath}' (${page.pattern}) is never used: '${used.componentPath}' (${used.pattern}) matches the same URLs first. Remove one of them.`,
495+
);
496+
}
497+
}
498+
}
499+
408500
// Same leading-script rule as @void/md's extractScript: a markdown page may open
409501
// with a <script> block, and its frontmatter starts right after that block.
410502
const LEADING_SCRIPT_RE = /^<script\b[^>]*>[\s\S]*?<\/script>\s*\n?/;

‎packages/void/src/pages/url.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,50 @@ export function isSameOrigin(url: URL, origin: string): boolean {
66
}
77
return url.origin === origin;
88
}
9+
10+
function isHexDigit(char: string | undefined): boolean {
11+
return (
12+
char !== undefined &&
13+
((char >= '0' && char <= '9') || (char >= 'a' && char <= 'f') || (char >= 'A' && char <= 'F'))
14+
);
15+
}
16+
17+
function tryDecodeURI(text: string): string {
18+
try {
19+
return decodeURI(text);
20+
} catch {
21+
return text;
22+
}
23+
}
24+
25+
/**
26+
* The path a page is matched on, decoded the way the server (Hono's getPath)
27+
* decodes it: `/caf%C3%A9` becomes `/café`, while `%25`, reserved escapes such
28+
* as `%2F` and malformed escapes stay encoded.
29+
*/
30+
export function decodePagePath(path: string): string {
31+
if (!path.includes('%')) {
32+
return path;
33+
}
34+
const escaped = path.split('%25').join('%2525');
35+
try {
36+
return decodeURI(escaped);
37+
} catch {
38+
// Decode each run of escapes on its own, keeping the runs that fail.
39+
}
40+
let decoded = '';
41+
let index = 0;
42+
while (index < escaped.length) {
43+
let end = index;
44+
while (escaped[end] === '%' && isHexDigit(escaped[end + 1]) && isHexDigit(escaped[end + 2])) {
45+
end += 3;
46+
}
47+
if (end === index) {
48+
decoded += escaped[index++];
49+
} else {
50+
decoded += tryDecodeURI(escaped.slice(index, end));
51+
index = end;
52+
}
53+
}
54+
return decoded;
55+
}

‎packages/void/src/plugin-routing.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -680,7 +680,26 @@ export function routingPlugin(root: string, options: RoutingPluginOptions): Plug
680680
// Re-scan and invalidate the virtual module in ALL environments.
681681
// The route table virtual module lives in the workerd environment,
682682
// not the client — so we must iterate all environments.
683-
await rescan(true);
683+
try {
684+
await rescan(true);
685+
} catch (error) {
686+
// Keep the last good route table; the next change rescans.
687+
const message = error instanceof Error ? error.message : String(error);
688+
// An adapter's pages plugin rescans page adds, removals and .md saves
689+
// and logs the same page error, so log it once.
690+
const adapterLogs =
691+
message.startsWith('pages:') &&
692+
isInDirectory(normalizedPath, normalizedPagesDir) &&
693+
(event !== 'change' || normalizedPath.endsWith('.md')) &&
694+
resolvedConfig?.plugins?.some((plugin) => plugin.name.endsWith(':pages')) === true;
695+
if (!adapterLogs) {
696+
devServer.config.logger.error(message, {
697+
error: error instanceof Error ? error : undefined,
698+
timestamp: true,
699+
});
700+
}
701+
return;
702+
}
684703
resolveAuthCompileOptions();
685704
const shouldRestartForAssetRouting =
686705
event !== 'change' &&

‎packages/void/src/router/scan.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,15 @@ export async function scanRoutes(
7575
output?: 'server' | 'static';
7676
paths: ProjectPaths;
7777
inventory: Omit<ScanResult, 'pages'>;
78+
checkPageRoutes?: boolean;
7879
},
7980
): Promise<ScanResult> {
8081
const { paths, inventory } = options;
81-
const pages = await scanPages(root, { output: options.output, paths });
82+
const pages = await scanPages(root, {
83+
output: options.output,
84+
paths,
85+
checkRoutes: options.checkPageRoutes,
86+
});
8287
const routes = inventory.routes
8388
.map((route) => ({
8489
...route,

0 commit comments

Comments
 (0)