Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/client-api-router.md
Original file line number Diff line number Diff line change
@@ -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.
28 changes: 28 additions & 0 deletions apps/tests/src/routes/treeshaking/api-router.server.test.ts
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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);
});
});
84 changes: 84 additions & 0 deletions packages/start/src/server/api-routes.ts
Original file line number Diff line number Diff line change
@@ -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<string, { route: Route }>,
),
});

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<string, any>;
handler: {
import: () => Promise<Record<string, (e: FetchEvent) => Promise<any>>>;
};
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;
}
2 changes: 1 addition & 1 deletion packages/start/src/server/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
81 changes: 2 additions & 79 deletions packages/start/src/server/routes.ts
Original file line number Diff line number Diff line change
@@ -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[];
Expand All @@ -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));
Expand Down Expand Up @@ -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<string, { route: Route }>,
),
});

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<string, any>;
handler: {
import: () => Promise<Record<string, (e: FetchEvent) => Promise<any>>>;
};
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;
}
Loading