diff --git a/packages/docs/src/pages/learn/FileSystemRouting.mdx b/packages/docs/src/pages/learn/FileSystemRouting.mdx index e632714..49f4765 100644 --- a/packages/docs/src/pages/learn/FileSystemRouting.mdx +++ b/packages/docs/src/pages/learn/FileSystemRouting.mdx @@ -79,6 +79,8 @@ The adapter fails the build with a clear error instead of producing broken route - **Parallel route slots** (`@slot`) and **intercepting routes** (`(.)segment`) — these Next.js features are not supported. - **Conflicting pages** — two pages resolving to the same route, such as `(a)/foo/page.tsx` + `(b)/foo/page.tsx`, or sibling dynamic pages with different param names (`[a]` + `[b]`). - **Duplicate files** — two page (or layout) files in the same directory, such as `page.tsx` next to `page.jsx`. +- **Segment names routes cannot match** — param names may only contain letters, digits, `_`, and `$` (so `[foo-bar]` is rejected); the same param name may appear only once on a route path; and static directory names must not contain URL-pattern characters (`:`, `*`, `?`, `+`, parentheses, braces, or backslash). +- **Route modules without a default export** — a `page.tsx` or `layout.tsx` that does not `export default` a component would silently render an empty page. Multiple root layouts via route groups (e.g. `(marketing)/layout.tsx` and `(shop)/layout.tsx`) are supported. @@ -122,6 +124,8 @@ This generates `blog/hello.html` and `blog/world.html`. Each page component rece A dynamic route **must** export `generateStaticParams`; the build fails otherwise. A static site can only serve pages that were enumerated at build time, so a dynamic route without it would produce no output. +Param values must be non-empty strings that stay within their URL segment: a regular param value must not contain `/`, and no value may contain `.` or `..` segments, `?`, or `#`. A catch-all param value may contain `/` to span multiple segments, but not leading, trailing, or repeated slashes. The build fails on any other value, since it would generate a page that its own route can never match (or a file outside the output directory). + Because `generateStaticParams` runs on the server at build time, a page module that exports it cannot be marked `"use client"`. If the page body needs to be a Client Component, move it into a separate `"use client"` module and re-export it from the page: ```tsx diff --git a/packages/static/src/fs-routes/nextAdapter.test.ts b/packages/static/src/fs-routes/nextAdapter.test.ts index e8646c0..5287679 100644 --- a/packages/static/src/fs-routes/nextAdapter.test.ts +++ b/packages/static/src/fs-routes/nextAdapter.test.ts @@ -202,6 +202,47 @@ describe("nextRoutes adapter", () => { ).toThrow(/Intercepting routes/); }); + it("rejects param names URL patterns cannot express", () => { + const adapter = nextRoutes(); + expect(() => + adapter.buildRoutes(makeFiles(["blog/[foo-bar]/page.tsx"])), + ).toThrow(/Invalid param name "foo-bar"/); + expect(() => + adapter.buildRoutes(makeFiles(["docs/[...foo.bar]/page.tsx"])), + ).toThrow(/Invalid param name "foo\.bar"/); + }); + + it("allows param names with letters, digits, underscore, and dollar", () => { + const adapter = nextRoutes(); + const tree = adapter.buildRoutes(makeFiles(["u/[$user_1]/page.tsx"])); + expect(simplify(tree)).toEqual([ + { path: "/u/:$user_1", page: true, id: "u/[$user_1]/page.tsx" }, + ]); + }); + + it("rejects a param name used twice on one route path", () => { + const adapter = nextRoutes(); + expect(() => + adapter.buildRoutes(makeFiles(["[slug]/x/[slug]/page.tsx"])), + ).toThrow(/Duplicate param name "slug"/); + // Also across a layout boundary, where the inner value would shadow + // the outer one. + expect(() => + adapter.buildRoutes( + makeFiles(["[id]/layout.tsx", "[id]/sub/[id]/page.tsx"]), + ), + ).toThrow(/Duplicate param name "id"/); + }); + + it("rejects static directory names containing URL pattern characters", () => { + const adapter = nextRoutes(); + for (const dir of ["a+b", "a?b", "a*b", "a:b", "a(b)c", "a{b}"]) { + expect(() => adapter.buildRoutes(makeFiles([`${dir}/page.tsx`]))).toThrow( + /special meaning in URL patterns/, + ); + } + }); + it("rejects two pages resolving to the same route via route groups", () => { const adapter = nextRoutes(); expect(() => diff --git a/packages/static/src/fs-routes/nextAdapter.ts b/packages/static/src/fs-routes/nextAdapter.ts index 9ebbdb5..4f57590 100644 --- a/packages/static/src/fs-routes/nextAdapter.ts +++ b/packages/static/src/fs-routes/nextAdapter.ts @@ -57,9 +57,24 @@ function classify( return undefined; } +/** + * Param names FUNSTACK Router (via URLPattern) can express. Anything else — + * e.g. `[foo-bar]` — is parsed by URLPattern as a shorter param followed by + * literal text, silently producing a route that never matches its pages. + */ +const VALID_PARAM_NAME = /^[A-Za-z0-9_$]+$/; + +/** + * Characters with special meaning in URLPattern pathname patterns. A static + * directory name containing one would either fail URLPattern construction at + * match time (`?`, `+`) or silently match the wrong URLs (`:`, `*`, `(`…). + */ +const URL_PATTERN_SPECIAL_CHARS = /[:*?+(){}\\]/; + /** * Rejects directory segments using Next.js syntaxes that this adapter does not - * support, so they fail loudly instead of silently producing broken routes. + * support, and segments FUNSTACK Router's URL patterns cannot express, so they + * fail loudly instead of silently producing broken routes. */ function validateSegment(segment: string, filePath: string): void { if (/^\[\[.*\]\]$/.test(segment)) { @@ -78,6 +93,28 @@ function validateSegment(segment: string, filePath: string): void { `Intercepting routes ("${segment}" in "${filePath}") are not supported.`, ); } + // Route groups do not reach the URL, so their names are unconstrained. + if (segment.startsWith("(") && segment.endsWith(")")) { + return; + } + const dynamic = /^\[(?:\.\.\.)?(.+)\]$/.exec(segment); + if (dynamic) { + if (!VALID_PARAM_NAME.test(dynamic[1]!)) { + throw new Error( + `Invalid param name "${dynamic[1]}" ("${segment}" in "${filePath}"). ` + + `Param names may only contain letters, digits, "_", and "$".`, + ); + } + return; + } + const special = URL_PATTERN_SPECIAL_CHARS.exec(segment); + if (special) { + throw new Error( + `Directory name "${segment}" (in "${filePath}") contains "${special[0]}", ` + + `which has a special meaning in URL patterns and cannot be routed. ` + + `Rename the directory.`, + ); + } } /** @@ -254,8 +291,6 @@ export function nextRoutes(options: NextRoutesOptions = {}): FsRoutesAdapter { name: "next", buildRoutes(files: FsRouteFile[]): FsRouteTreeNode[] { const root: TrieNode = { segment: "", children: new Map() }; - // Route position each page/layout occupies, with dynamic segments - // normalized so that e.g. `[a]` and `[b]` at the same position conflict. // Exact directory each page/layout file lives in, to detect duplicate // files for the same node (e.g. `page.tsx` next to `page.jsx`). const filesByDir = new Map(); @@ -268,8 +303,22 @@ export function nextRoutes(options: NextRoutesOptions = {}): FsRoutesAdapter { const { dirs, base } = splitFilePath(file.filePath); const kind = classify(base, pageFileName, layoutFileName); if (!kind) continue; + const seenParamNames = new Set(); for (const segment of dirs) { validateSegment(segment, file.filePath); + const dynamic = /^\[(?:\.\.\.)?(.+)\]$/.exec(segment); + if (dynamic) { + // A param name used twice on one path either fails URLPattern + // construction (within one route) or shadows the outer value + // (across a layout boundary), so reject it up front. + if (seenParamNames.has(dynamic[1]!)) { + throw new Error( + `Duplicate param name "${dynamic[1]}" in "${file.filePath}": ` + + `a route may use each param name only once.`, + ); + } + seenParamNames.add(dynamic[1]!); + } } const dirKey = `${kind} ${dirs.join("/")}`; const sameDir = filesByDir.get(dirKey); diff --git a/packages/static/src/fs-routes/runtime.test.ts b/packages/static/src/fs-routes/runtime.test.ts index 8c5235d..cd21179 100644 --- a/packages/static/src/fs-routes/runtime.test.ts +++ b/packages/static/src/fs-routes/runtime.test.ts @@ -224,4 +224,19 @@ describe("createFsRoutesEntries route definitions", () => { expect(ids).toEqual(idsPerEntry[0]); } }); + + it("throws for a page module without a default export", async () => { + await expect(entriesFor({ "./pages/about/page.tsx": {} })).rejects.toThrow( + /page module "about\/page\.tsx" has no default export/, + ); + }); + + it("throws for a layout module without a default export", async () => { + await expect( + entriesFor({ + "./pages/layout.tsx": { notDefault: () => null }, + "./pages/page.tsx": { default: () => null }, + }), + ).rejects.toThrow(/layout module "layout\.tsx" has no default export/); + }); }); diff --git a/packages/static/src/fs-routes/runtime.tsx b/packages/static/src/fs-routes/runtime.tsx index 390dc6c..f498a42 100644 --- a/packages/static/src/fs-routes/runtime.tsx +++ b/packages/static/src/fs-routes/runtime.tsx @@ -3,7 +3,6 @@ import { Router } from "@funstack/router"; import type { RouteDefinition } from "@funstack/router/server"; import type { FsRootComponent, - FsRouteComponentProps, FsRouteModule, FsRouteObject, FsRoutesAdapter, @@ -99,6 +98,30 @@ interface NodeMeta { chunks: Record; } +/** + * Rejects route modules without a default export. Rendering would silently + * skip the missing component (producing a blank page, or a pass-through + * layout), so a typo'd or forgotten export must fail the build instead. + */ +function validateRouteModules(nodes: FsRouteTreeNode[]): void { + for (const node of nodes) { + if (node.module.default === undefined) { + const kind = node.page ? "page" : "layout"; + const which = + node.filePath === undefined + ? `for route "${node.path ?? "(pathless)"}"` + : `"${node.filePath}"`; + throw new Error( + `Route ${kind} module ${which} has no default export. ` + + `Page and layout modules must \`export default\` a React component.`, + ); + } + if (node.children) { + validateRouteModules(node.children); + } + } +} + function buildNodeMetas( nodes: FsRouteTreeNode[], inheritedParamNames: string[], @@ -162,8 +185,7 @@ function registerChunks( } for (const [node, nodeCombos] of combos) { const meta = metas.get(node)!; - const Component = node.module - .default as ComponentType; + const Component = node.module.default!; for (const [key, params] of nodeCombos) { const element = createElement(Component, { params, route: meta.route }); meta.chunks[key] = host.registerChunk( @@ -233,10 +255,10 @@ export function createFsRoutesEntriesWithHost( if (pageChain.has(node)) { const params = pickParams(pageParams, meta.paramNames); slotProps.initialKey = paramsKey(meta.paramNames, pageParams); - slotProps.initial = createElement( - Component as React.ComponentType, - { params, route: meta.route }, - ); + slotProps.initial = createElement(Component, { + params, + route: meta.route, + }); } definition.component = createElement(host.RouteSlot, slotProps); } @@ -281,6 +303,7 @@ export function createFsRoutesEntriesWithHost( }; const files = modulesToRouteFiles(modules, base, warn); const tree = adapter.buildRoutes(files); + validateRouteModules(tree); const pages = await collectStaticPaths(tree); const metas = new Map(); buildNodeMetas(tree, [], "", metas); diff --git a/packages/static/src/fs-routes/tree.test.ts b/packages/static/src/fs-routes/tree.test.ts index c8e2d93..cc4c281 100644 --- a/packages/static/src/fs-routes/tree.test.ts +++ b/packages/static/src/fs-routes/tree.test.ts @@ -144,6 +144,95 @@ describe("collectStaticPaths", () => { ); }); + it("throws when a non-catch-all value contains a slash", async () => { + const tree: FsRouteTreeNode[] = [ + { + path: "/blog/:slug", + page: true, + module: pageModule(() => [{ slug: "a/b" }]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow(/"a\/b".*catch-all/); + }); + + it("throws for an empty param value", async () => { + const tree: FsRouteTreeNode[] = [ + { + path: "/blog/:slug", + page: true, + module: pageModule(() => [{ slug: "" }]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow(/empty value/); + }); + + it("throws for an empty catch-all value, suggesting a parent page", async () => { + const tree: FsRouteTreeNode[] = [ + { + path: "/docs/:slug*", + page: true, + module: pageModule(() => [{ slug: "" }]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow( + /parent route instead/, + ); + }); + + it("throws for a non-string param value", async () => { + const tree: FsRouteTreeNode[] = [ + { + path: "/blog/:id", + page: true, + module: pageModule(() => [ + { id: 5 } as unknown as Record, + ]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow( + /returned a number.*"id"/, + ); + }); + + it("throws for a catch-all value with leading, trailing, or repeated slashes", async () => { + for (const slug of ["/a", "a/", "a//b"]) { + const tree: FsRouteTreeNode[] = [ + { + path: "/docs/:slug*", + page: true, + module: pageModule(() => [{ slug }]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow(/slashes/); + } + }); + + it('throws for param values containing "." or ".." segments', async () => { + for (const slug of ["..", ".", "a/../b"]) { + const tree: FsRouteTreeNode[] = [ + { + path: "/docs/:slug*", + page: true, + module: pageModule(() => [{ slug }]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow(/"\."/); + } + }); + + it('throws for param values containing "?" or "#"', async () => { + for (const slug of ["a?b", "a#b"]) { + const tree: FsRouteTreeNode[] = [ + { + path: "/blog/:slug", + page: true, + module: pageModule(() => [{ slug }]), + }, + ]; + await expect(collectStaticPaths(tree)).rejects.toThrow(/URL path/); + } + }); + it("throws when generateStaticParams is missing a param value", async () => { const tree: FsRouteTreeNode[] = [ { diff --git a/packages/static/src/fs-routes/tree.ts b/packages/static/src/fs-routes/tree.ts index 10faeb1..6166887 100644 --- a/packages/static/src/fs-routes/tree.ts +++ b/packages/static/src/fs-routes/tree.ts @@ -108,6 +108,71 @@ function inFile(filePath: string | undefined): string { return filePath === undefined ? "" : ` ("${filePath}")`; } +/** + * Validates one param value returned by `generateStaticParams()` and returns + * it. Values are substituted verbatim into URL paths (and output file + * paths), so a value the route's own URLPattern cannot match back — or one + * that escapes the output directory — must fail the build instead of + * producing a silently broken page. + */ +function substituteParamValue( + segment: string, + params: Record, + routePath: string, + filePath: string | undefined, +): string { + const name = paramName(segment); + const where = `generateStaticParams() for "${routePath}"${inFile(filePath)}`; + const value: unknown = params[name]; + if (value === undefined) { + throw new Error(`${where} is missing a value for param "${name}".`); + } + if (typeof value !== "string") { + throw new Error( + `${where} returned a ${typeof value} for param "${name}". ` + + `Param values must be strings.`, + ); + } + const isCatchAll = segment.endsWith("*"); + if (value === "") { + throw new Error( + isCatchAll + ? `${where} returned an empty value for catch-all param "${name}". ` + + `A zero-segment catch-all page cannot be statically served; ` + + `create a page for the parent route instead.` + : `${where} returned an empty value for param "${name}".`, + ); + } + const parts = value.split("/"); + if (!isCatchAll && parts.length > 1) { + throw new Error( + `${where} returned "${value}" for param "${name}", which contains "/". ` + + `Only a catch-all param (":${name}*") may span multiple URL segments.`, + ); + } + for (const part of parts) { + if (part === "") { + throw new Error( + `${where} returned "${value}" for catch-all param "${name}". ` + + `Values must not contain leading, trailing, or repeated slashes.`, + ); + } + if (part === "." || part === "..") { + throw new Error( + `${where} returned "${value}" for param "${name}". ` + + `Values must not contain "." or ".." segments.`, + ); + } + } + if (value.includes("?") || value.includes("#")) { + throw new Error( + `${where} returned "${value}" for param "${name}". ` + + `Values must not contain "?" or "#", which cannot appear in a URL path.`, + ); + } + return value; +} + async function addPagesForLeaf( segments: string[], module: FsRouteModule, @@ -142,17 +207,11 @@ async function addPagesForLeaf( } const paramSets = await generate(); + const routePath = segmentsToUrl(segments); for (const params of paramSets) { const concreteSegments = segments.map((segment) => { if (!isDynamicSegment(segment)) return segment; - const name = paramName(segment); - const value = params[name]; - if (value === undefined) { - throw new Error( - `generateStaticParams() for "${segmentsToUrl(segments)}"${inFile(filePath)} is missing a value for param "${name}".`, - ); - } - return value; + return substituteParamValue(segment, params, routePath, filePath); }); pages.push({ urlPath: segmentsToUrl(concreteSegments), params, chain }); } diff --git a/packages/static/src/fs-routes/types.ts b/packages/static/src/fs-routes/types.ts index bfefa43..674d51f 100644 --- a/packages/static/src/fs-routes/types.ts +++ b/packages/static/src/fs-routes/types.ts @@ -54,11 +54,12 @@ export interface FsRouteComponentProps< * for dynamic routes (modeled after Next.js). */ export interface FsRouteModule { - /** The component for this page or layout. */ - default?: - | ComponentType - | ComponentType<{ params: Record }> - | ComponentType; + /** + * The component for this page or layout. Components taking fewer props + * (or none) are assignable; the framework always passes + * {@link FsRouteComponentProps}. + */ + default?: ComponentType; /** * Function used to statically generate a dynamic route. Required for pages * whose route contains a dynamic segment; the build fails without it, since @@ -69,8 +70,11 @@ export interface FsRouteModule { * module and re-export it as `default` instead. * * Returns the list of concrete params to pre-render. Each entry maps every - * dynamic param name in the route's path to a concrete string value. For a - * catch-all segment, the value may contain slashes. + * dynamic param name in the route's path to a concrete string value. Values + * must be non-empty and must not contain `.` or `..` segments, `?`, or + * `#`. Only a catch-all segment's value may contain slashes (but not + * leading, trailing, or repeated ones). The build fails on any other + * value, which would generate a page its own route cannot match. */ generateStaticParams?: () => MaybePromise>>; [key: string]: unknown; diff --git a/packages/static/src/plugin/index.test.ts b/packages/static/src/plugin/index.test.ts index 78f0955..3e5b2cc 100644 --- a/packages/static/src/plugin/index.test.ts +++ b/packages/static/src/plugin/index.test.ts @@ -136,6 +136,24 @@ describe("virtual module path escaping", () => { expect(code).toContain('import adapter from "/pro\\"ject/src/adapter.ts";'); }); + it("rejects an fsRoutes.dir outside the Vite root", () => { + for (const dir of ["../pages", "."]) { + expect(() => + loadVirtualModule( + { + fsRoutes: { + dir, + root: "./src/root.tsx", + adapter: "./src/adapter.ts", + }, + }, + "/project", + "\0virtual:funstack/entries", + ), + ).toThrow(/must be a subdirectory of the Vite root/); + } + }); + it("escapes the clientInit path in the client-init module", () => { const code = loadVirtualModule( { diff --git a/packages/static/src/plugin/index.ts b/packages/static/src/plugin/index.ts index ae9a207..5b64c89 100644 --- a/packages/static/src/plugin/index.ts +++ b/packages/static/src/plugin/index.ts @@ -221,7 +221,18 @@ export default function funstackStatic( const relativeDir = normalizePath( path.relative(config.root, resolvedDir), ); - const globBase = `/${relativeDir.replace(/^\.?\/?/, "").replace(/\/$/, "")}`; + if ( + relativeDir === "" || + relativeDir === "." || + relativeDir.startsWith("..") || + path.isAbsolute(relativeDir) + ) { + throw new Error( + `[funstack] fsRoutes.dir ("${fsRoutes.dir}") must be a subdirectory ` + + `of the Vite root ("${config.root}").`, + ); + } + const globBase = `/${relativeDir}`; // The adapter may be a bare module specifier (e.g. the built-in // `@funstack/static/fs-routes/next-adapter`) or a path to a local module. // Resolve only the latter against the Vite root.