diff --git a/.changeset/client-api-router.md b/.changeset/client-api-router.md new file mode 100644 index 000000000..e1c41e4e1 --- /dev/null +++ b/.changeset/client-api-router.md @@ -0,0 +1,5 @@ +--- +"@solidjs/start": patch +--- + +Keep the API route matcher out of the client bundle. It is only used on the server, but the client loaded it with the page routes, which added about 2.5 KB of unused code. Routes that only export an `OPTIONS` handler are now matched as API routes. diff --git a/apps/tests/src/routes/treeshaking/api-router.server.test.ts b/apps/tests/src/routes/treeshaking/api-router.server.test.ts new file mode 100644 index 000000000..1ef7be769 --- /dev/null +++ b/apps/tests/src/routes/treeshaking/api-router.server.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { getBuildOutputDirs, getFiles, readFileContent } from "~/utils/build-output-utils"; + +// Regression test for https://github.com/solidjs/solid-start/issues/2361. +// The client imports `pageRoutes`, which used to share a module with the API +// router. Building that router at module scope kept it and radix3 in the +// client bundle, even though the client has no API routes to match. +const MARKER = "Optional parameters are not supported in API routes"; + +describe("API router is excluded from the client bundle", () => { + it("ships the API router to the server only", async () => { + const { serverOutputRoot, clientOutputRoot } = getBuildOutputDirs(); + + const serverFiles = await getFiles(serverOutputRoot, /\.(m?js)$/); + const serverContents = await Promise.all(serverFiles.map(readFileContent)); + expect( + serverContents.some(content => content.includes(MARKER)), + "API router not found in the server output", + ).toBe(true); + + const clientFiles = await getFiles(clientOutputRoot, /\.(m?js)$/); + expect(clientFiles.length, "No client JS files found").toBeGreaterThan(0); + for (const file of clientFiles) { + const content = await readFileContent(file); + expect(content, `API router leaked into: ${file}`).not.toContain(MARKER); + } + }); +}); diff --git a/packages/start/src/server/routes.spec.ts b/packages/start/src/server/api-routes.spec.ts similarity index 82% rename from packages/start/src/server/routes.spec.ts rename to packages/start/src/server/api-routes.spec.ts index 367601bc9..50919d7ee 100644 --- a/packages/start/src/server/routes.spec.ts +++ b/packages/start/src/server/api-routes.spec.ts @@ -14,10 +14,11 @@ vi.mock("solid-start:routes", () => ({ }, { path: "/spa/*404", page: true, $GET: GET }, { path: "/api/*rest", $GET: GET }, + { path: "/cors", $OPTIONS: GET }, ], })); -const { matchAPIRoute } = await import("./routes.ts"); +const { matchAPIRoute } = await import("./api-routes.ts"); describe("matchAPIRoute", () => { it("marks a route with a page and an API handler as a page", () => { @@ -31,4 +32,8 @@ describe("matchAPIRoute", () => { it("does not mark an API-only route as a page", () => { expect(matchAPIRoute("/api/missing", "GET")?.isPage).toBe(false); }); + + it("matches a route that only exports OPTIONS", () => { + expect(matchAPIRoute("/cors", "OPTIONS")?.handler).toBe(GET); + }); }); diff --git a/packages/start/src/server/api-routes.ts b/packages/start/src/server/api-routes.ts new file mode 100644 index 000000000..db84d03d9 --- /dev/null +++ b/packages/start/src/server/api-routes.ts @@ -0,0 +1,84 @@ +// @ts-expect-error +import fileRoutes from "solid-start:routes"; +import { createRouter } from "radix3"; + +import type { Route } from "./routes.ts"; +import type { FetchEvent } from "./types.ts"; + +// The API router lives apart from `pageRoutes` so the client bundle, which +// imports `pageRoutes`, doesn't include radix3 or build this router. +const router = createRouter({ + routes: (fileRoutes as unknown as Route[]).reduce( + (memo, route) => { + if (!containsHTTP(route)) return memo; + const path = route.path + .replace(/\([^)/]+\)/g, "") + .replace(/\/+/g, "/") + .replace(/\*([^/]*)/g, (_, m) => `**:${m}`) + .split("/") + .map(s => (s.startsWith(":") || s.startsWith("*") ? s : encodeURIComponent(s))) + .join("/"); + if (/:[^/]*\?/g.test(path)) { + throw new Error(`Optional parameters are not supported in API routes: ${path}`); + } + if (memo[path]) { + throw new Error( + `Duplicate API routes for "${path}" found at "${memo[path]!.route.path}" and "${ + route.path + }"`, + ); + } + memo[path] = { route }; + return memo; + }, + {} as Record, + ), +}); + +function containsHTTP(route: Route) { + return ( + route["$HEAD"] || + route["$GET"] || + route["$POST"] || + route["$PUT"] || + route["$PATCH"] || + route["$DELETE"] || + route["$OPTIONS"] + ); +} + +export function matchAPIRoute( + path: string, + method: string, +): + | { + params?: Record; + handler: { + import: () => Promise Promise>>; + }; + isPage: boolean; + } + | undefined { + const match = router.lookup(path); + if (match && match.route) { + const route = match.route; + + // Find the appropriate handler for the HTTP method + const handler = method === "HEAD" ? route.$HEAD || route.$GET : route[`$${method}`]; + + if (handler === undefined) return; + + // Check if this is a page route. SPA builds leave page components out of + // the server routes, so this can't depend on `$component`. + const isPage = route.page === true; + + // Return comprehensive route information + return { + handler, + params: match.params, + isPage, + }; + } + + return undefined; +} diff --git a/packages/start/src/server/handler.ts b/packages/start/src/server/handler.ts index 060f6d3e6..a705537d6 100644 --- a/packages/start/src/server/handler.ts +++ b/packages/start/src/server/handler.ts @@ -8,7 +8,7 @@ import { createRoutes } from "../router.tsx"; import { decorateHandler, decorateMiddleware } from "./fetchEvent.ts"; import { sendEarlyHints } from "./early-hints.ts"; import { getSsrManifest } from "./manifest/ssr-manifest.ts"; -import { matchAPIRoute } from "./routes.ts"; +import { matchAPIRoute } from "./api-routes.ts"; import { handleServerFunction } from "../fns/handler.ts"; import type { APIEvent, FetchEvent, HandlerOptions, PageEvent, StartHandler } from "./types.ts"; import { getExpectedRedirectStatus } from "./util.ts"; diff --git a/packages/start/src/server/routes.ts b/packages/start/src/server/routes.ts index 4521cff8e..157404fd7 100644 --- a/packages/start/src/server/routes.ts +++ b/packages/start/src/server/routes.ts @@ -1,10 +1,7 @@ // @ts-expect-error import fileRoutes from "solid-start:routes"; -import { createRouter } from "radix3"; -import type { FetchEvent } from "./types.ts"; - -interface Route { +export interface Route { path: string; id: string; children?: Route[]; @@ -16,6 +13,7 @@ interface Route { $PUT?: any; $PATCH?: any; $DELETE?: any; + $OPTIONS?: any; } export const pageRoutes = defineRoutes((fileRoutes as unknown as Route[]).filter(o => o.page)); @@ -50,78 +48,3 @@ function defineRoutes(fileRoutes: Route[]) { return processRoute(prevRoutes, route, route.path, route.path); }, []); } - -const router = createRouter({ - routes: (fileRoutes as unknown as Route[]).reduce( - (memo, route) => { - if (!containsHTTP(route)) return memo; - const path = route.path - .replace(/\([^)/]+\)/g, "") - .replace(/\/+/g, "/") - .replace(/\*([^/]*)/g, (_, m) => `**:${m}`) - .split("/") - .map(s => (s.startsWith(":") || s.startsWith("*") ? s : encodeURIComponent(s))) - .join("/"); - if (/:[^/]*\?/g.test(path)) { - throw new Error(`Optional parameters are not supported in API routes: ${path}`); - } - if (memo[path]) { - throw new Error( - `Duplicate API routes for "${path}" found at "${memo[path]!.route.path}" and "${ - route.path - }"`, - ); - } - memo[path] = { route }; - return memo; - }, - {} as Record, - ), -}); - -function containsHTTP(route: Route) { - return ( - route["$HEAD"] || - route["$GET"] || - route["$POST"] || - route["$PUT"] || - route["$PATCH"] || - route["$DELETE"] - ); -} - -export function matchAPIRoute( - path: string, - method: string, -): - | { - params?: Record; - handler: { - import: () => Promise Promise>>; - }; - isPage: boolean; - } - | undefined { - const match = router.lookup(path); - if (match && match.route) { - const route = match.route; - - // Find the appropriate handler for the HTTP method - const handler = method === "HEAD" ? route.$HEAD || route.$GET : route[`$${method}`]; - - if (handler === undefined) return; - - // Check if this is a page route. SPA builds leave page components out of - // the server routes, so this can't depend on `$component`. - const isPage = route.page === true; - - // Return comprehensive route information - return { - handler, - params: match.params, - isPage, - }; - } - - return undefined; -}