-
Notifications
You must be signed in to change notification settings - Fork 218
fix: vinext start for Cloudflare App Router worker exports #460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import { normalizePath } from "./normalize-path.js"; | |
| import { hasBasePath, stripBasePath } from "../utils/base-path.js"; | ||
| import { computeLazyChunks } from "../index.js"; | ||
| import { manifestFileWithBase } from "../utils/manifest-paths.js"; | ||
| import type { ExecutionContextLike } from "../shims/request-context.js"; | ||
|
|
||
| /** Convert a Node.js IncomingMessage into a ReadableStream for Web Request body. */ | ||
| function readNodeStream(req: IncomingMessage): ReadableStream<Uint8Array> { | ||
|
|
@@ -508,11 +509,47 @@ interface AppRouterServerOptions { | |
| compress: boolean; | ||
| } | ||
|
|
||
| interface WorkerAppRouterEntry { | ||
| fetch(request: Request, env?: unknown, ctx?: ExecutionContextLike): Promise<Response> | Response; | ||
| } | ||
|
|
||
| function createNodeExecutionContext(): ExecutionContextLike { | ||
| return { | ||
| waitUntil(promise: Promise<unknown>) { | ||
| // Node doesn't provide a Workers lifecycle, but we still attach a | ||
| // rejection handler so background waitUntil work doesn't surface as an | ||
| // unhandled rejection when a Worker-style entry is used with vinext start. | ||
| void Promise.resolve(promise).catch(() => {}); | ||
| }, | ||
| passThroughOnException() {}, | ||
| }; | ||
| } | ||
|
|
||
| function resolveAppRouterHandler(entry: unknown): (request: Request) => Promise<Response> { | ||
| if (typeof entry === "function") { | ||
| return (request) => Promise.resolve(entry(request)); | ||
| } | ||
|
|
||
| if (entry && typeof entry === "object" && "fetch" in entry) { | ||
| const workerEntry = entry as WorkerAppRouterEntry; | ||
| if (typeof workerEntry.fetch === "function") { | ||
| return (request) => | ||
| Promise.resolve(workerEntry.fetch(request, undefined, createNodeExecutionContext())); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call creating a fresh context per request rather than sharing one. This matches Cloudflare Workers semantics where each request gets its own |
||
| } | ||
| } | ||
|
|
||
| console.error( | ||
| "[vinext] App Router entry must export either a default handler function or a Worker-style default export with fetch()", | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| /** | ||
| * Start the App Router production server. | ||
| * | ||
| * The RSC entry (dist/server/index.js) exports a default handler function: | ||
| * handler(request: Request) → Promise<Response> | ||
| * The App Router entry (dist/server/index.js) can export either: | ||
| * - a default handler function: handler(request: Request) → Promise<Response> | ||
| * - a Worker-style object: { fetch(request, env, ctx) → Promise<Response> } | ||
| * | ||
| * This handler already does everything: route matching, RSC rendering, | ||
| * SSR HTML generation (via import("./ssr/index.js")), route handlers, | ||
|
|
@@ -541,12 +578,7 @@ async function startAppRouterServer(options: AppRouterServerOptions) { | |
|
|
||
| // Import the RSC handler (use file:// URL for reliable dynamic import) | ||
| const rscModule = await import(pathToFileURL(rscEntryPath).href); | ||
| const rscHandler: (request: Request) => Promise<Response> = rscModule.default; | ||
|
|
||
| if (typeof rscHandler !== "function") { | ||
| console.error("[vinext] RSC entry does not export a default handler function"); | ||
| process.exit(1); | ||
| } | ||
| const rscHandler = resolveAppRouterHandler(rscModule.default); | ||
|
|
||
| const server = createServer(async (req, res) => { | ||
| const url = req.url ?? "/"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1600,6 +1600,79 @@ describe("App Router Production server (startProdServer)", () => { | |
| }); | ||
| }); | ||
|
|
||
| describe("App Router Production server worker entry compatibility", () => { | ||
| it("accepts Worker-style default exports from dist/server/index.js", async () => { | ||
| const outDir = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-prod-worker-entry-")); | ||
| const serverDir = path.join(outDir, "server"); | ||
| fs.mkdirSync(serverDir, { recursive: true }); | ||
| fs.mkdirSync(path.join(outDir, "client"), { recursive: true }); | ||
| fs.writeFileSync(path.join(outDir, "package.json"), JSON.stringify({ type: "module" })); | ||
| fs.writeFileSync( | ||
| path.join(serverDir, "index.js"), | ||
| ` | ||
| export default { | ||
| async fetch(request, _env, ctx) { | ||
| ctx?.waitUntil(Promise.resolve("background")); | ||
| return new Response( | ||
| JSON.stringify({ | ||
| pathname: new URL(request.url).pathname, | ||
| hasWaitUntil: typeof ctx?.waitUntil === "function", | ||
| }), | ||
| { headers: { "content-type": "application/json" } }, | ||
| ); | ||
| }, | ||
| }; | ||
| `, | ||
| ); | ||
|
|
||
| const { startProdServer } = await import("../packages/vinext/src/server/prod-server.js"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since |
||
| const server = await startProdServer({ port: 0, outDir, noCompression: true }); | ||
| const addr = server.address(); | ||
| const port = typeof addr === "object" && addr ? addr.port : 0; | ||
|
|
||
| try { | ||
| const res = await fetch(`http://localhost:${port}/worker-test`); | ||
| expect(res.status).toBe(200); | ||
| expect(await res.json()).toEqual({ | ||
| pathname: "/worker-test", | ||
| hasWaitUntil: true, | ||
| }); | ||
| } finally { | ||
| server.close(); | ||
| fs.rmSync(outDir, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
|
|
||
| it("reports a clear error for unsupported app router entry shapes", async () => { | ||
| const outDir = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-prod-worker-invalid-")); | ||
| const serverDir = path.join(outDir, "server"); | ||
| fs.mkdirSync(serverDir, { recursive: true }); | ||
| fs.writeFileSync(path.join(outDir, "package.json"), JSON.stringify({ type: "module" })); | ||
| fs.writeFileSync(path.join(serverDir, "index.js"), "export default {};\n"); | ||
|
|
||
| const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); | ||
| const exitSpy = vi.spyOn(process, "exit").mockImplementation((( | ||
| code?: string | number | null, | ||
| ) => { | ||
| throw new Error(`process.exit(${code})`); | ||
| }) as never); | ||
|
|
||
| try { | ||
| const { startProdServer } = await import("../packages/vinext/src/server/prod-server.js"); | ||
| await expect(startProdServer({ port: 0, outDir, noCompression: true })).rejects.toThrow( | ||
| "process.exit(1)", | ||
| ); | ||
| expect(errorSpy).toHaveBeenCalledWith( | ||
| "[vinext] App Router entry must export either a default handler function or a Worker-style default export with fetch()", | ||
| ); | ||
| } finally { | ||
| errorSpy.mockRestore(); | ||
| exitSpy.mockRestore(); | ||
| fs.rmSync(outDir, { recursive: true, force: true }); | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Malformed percent-encoded URL regression tests — App Router dev server | ||
| // (covers entries/app-rsc-entry.ts generated RSC handler decodeURIComponent) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: silently swallowing rejections here is reasonable for
vinext start(which is a local/preview server, not a production deployment target), but it could mask real bugs during local debugging. Consider logging rejectedwaitUntilpromises atdebuglevel so developers can opt in to visibility:Entirely optional — the current behavior is defensible.