Skip to content
Merged
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
34 changes: 34 additions & 0 deletions packages/vinext/src/entries/app-rsc-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ export type AppRouterConfig = {
* `virtual:vinext-server-entry` when this flag is set.
*/
hasPagesDir?: boolean;
/** Exact public/ file routes, using normalized leading-slash pathnames. */
publicFiles?: string[];
};

/**
Expand Down Expand Up @@ -123,6 +125,7 @@ export function generateRscEntry(
const bodySizeLimit = config?.bodySizeLimit ?? 1 * 1024 * 1024;
const i18nConfig = config?.i18n ?? null;
const hasPagesDir = config?.hasPagesDir ?? false;
const publicFiles = config?.publicFiles ?? [];
// Build import map for all page and layout files
const imports: string[] = [];
const importMap: Map<string, string> = new Map();
Expand Down Expand Up @@ -845,6 +848,21 @@ function matchRoute(url) {
return _trieMatch(_routeTrie, urlParts);
}

function __createStaticFileSignal(pathname, _mwCtx) {
const headers = new Headers({
"x-vinext-static-file": encodeURIComponent(pathname),
});
if (_mwCtx.headers) {
for (const [key, value] of _mwCtx.headers) {
headers.append(key, value);
}
}
return new Response(null, {
status: _mwCtx.status ?? 200,
headers,
});
}

// matchPattern is kept for findIntercept (linear scan over small interceptLookup array).
function matchPattern(urlParts, patternParts) {
const params = Object.create(null);
Expand Down Expand Up @@ -1214,6 +1232,7 @@ const __i18nConfig = ${JSON.stringify(i18nConfig)};
const __configRedirects = ${JSON.stringify(redirects)};
const __configRewrites = ${JSON.stringify(rewrites)};
const __configHeaders = ${JSON.stringify(headers)};
const __publicFiles = new Set(${JSON.stringify(publicFiles)});
const __allowedOrigins = ${JSON.stringify(allowedOrigins)};

${generateDevOriginCheckCode(config?.allowedDevOrigins)}
Expand Down Expand Up @@ -1424,6 +1443,9 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
${
bp
? `
if (!hasBasePath(pathname, __basePath) && !pathname.startsWith("/__vinext/")) {
return new Response("Not Found", { status: 404 });
}
Comment on lines 1443 to +1448
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This basePath 404 guard is a meaningful behavioral change beyond "serve public files." Previously, requests without the basePath prefix could fall through to catch-all routes or middleware; now they get a hard 404.

This matches Next.js behavior and is probably correct, but it should be called out in the PR description since it changes observable behavior for any app with basePath configured. The /__vinext/ exception is good — internal endpoints need to bypass.

Worth noting: this also blocks middleware from seeing non-basePath requests. In Next.js, middleware's matcher config controls this, but here the 404 fires before middleware runs. This is consistent with how Next.js handles it (basePath rejection is early), but should be verified.

// Strip basePath prefix
pathname = stripBasePath(pathname, __basePath);
`
Expand Down Expand Up @@ -1781,6 +1803,18 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
}
}

// Serve public/ files as filesystem routes after middleware and before
// afterFiles/fallback rewrites, matching Next.js routing semantics.
if (
(request.method === "GET" || request.method === "HEAD") &&
!pathname.endsWith(".rsc") &&
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: pathname here (with .rsc suffix intact) vs cleanPathname (.rsc stripped) is a subtle distinction that's easy to misread. The logic is correct — pathname retains .rsc so the guard rejects RSC data fetches, while cleanPathname is the actual filesystem path to look up — but a brief inline comment would help future readers understand why two different variables are used in the same condition.

Suggested change
!pathname.endsWith(".rsc") &&
// Guard: pathname retains the .rsc suffix (if any) so we can reject RSC data fetches;
// cleanPathname has it stripped and is the actual filesystem path to match.
if (
(request.method === "GET" || request.method === "HEAD") &&
!pathname.endsWith(".rsc") &&
__publicFiles.has(cleanPathname)
) {

__publicFiles.has(cleanPathname)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the .rsc suffix check prevents RSC data fetches from being intercepted, which is correct. But there's no guard against POST requests to public file paths that happen to match (e.g., a form submission to /logo/logo.svg). The GET || HEAD check handles this — just confirming that's intentional and sufficient.

Also, cleanPathname here is post-middleware-rewrite, which means middleware can rewrite any path to serve a public file. This matches Next.js semantics (middleware rewrites are applied before filesystem route matching).

) {
setHeadersContext(null);
setNavigationContext(null);
return __createStaticFileSignal(cleanPathname, _mwCtx);
}

// Set navigation context for Server Components.
// Note: Headers context is already set by runWithRequestContext in the handler wrapper.
setNavigationContext({
Expand Down
57 changes: 56 additions & 1 deletion packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
bodySizeLimit: nextConfig?.serverActionsBodySizeLimit,
i18n: nextConfig?.i18n,
hasPagesDir,
publicFiles: scanPublicFileRoutes(root),
},
instrumentationPath,
);
Expand Down Expand Up @@ -4070,7 +4071,6 @@ function findFileWithExts(

/** Module-level cache for hasMdxFiles — avoids re-scanning per Vite environment. */
const _mdxScanCache = new Map<string, boolean>();

/**
* Check if the project has .mdx files in app/ or pages/ directories.
*/
Expand Down Expand Up @@ -4106,6 +4106,60 @@ function scanDirForMdx(dir: string): boolean {
return false;
}

function scanPublicFileRoutes(root: string): string[] {
const publicDir = path.join(root, "public");
const routes: string[] = [];
const visitedDirs = new Set<string>();

function walk(dir: string): void {
let realDir: string;
try {
realDir = fs.realpathSync(dir);
} catch {
return;
}
if (visitedDirs.has(realDir)) return;
visitedDirs.add(realDir);

const entries = fs.readdirSync(dir, { withFileTypes: true });
for (const entry of entries) {
const fullPath = path.join(dir, entry.name);
if (entry.isDirectory()) {
walk(fullPath);
continue;
}
if (entry.isSymbolicLink()) {
let stat: fs.Stats;
try {
stat = fs.statSync(fullPath);
} catch {
continue;
}
if (stat.isDirectory()) {
walk(fullPath);
continue;
}
if (!stat.isFile()) continue;
} else if (!entry.isFile()) {
continue;
}
const relativePath = path.relative(publicDir, fullPath).split(path.sep).join("/");
routes.push("/" + relativePath);
}
}

if (fs.existsSync(publicDir)) {
try {
walk(publicDir);
} catch {
// ignore unreadable dirs
}
}

routes.sort();
return routes;
}

// Public exports for static export
export { staticExportPages, staticExportApp } from "./build/static-export.js";
export type {
Expand All @@ -4132,6 +4186,7 @@ export { resolvePostcssStringPlugins as _resolvePostcssStringPlugins };
export { _postcssCache };
export { hasMdxFiles as _hasMdxFiles };
export { _mdxScanCache };
export { scanPublicFileRoutes as _scanPublicFileRoutes };
export { parseStaticObjectLiteral as _parseStaticObjectLiteral };
export { _findBalancedObject, _findCallEnd };
export { stripServerExports as _stripServerExports };
Expand Down
20 changes: 19 additions & 1 deletion packages/vinext/src/server/app-router-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,20 @@
// @ts-expect-error — virtual module resolved by vinext
import rscHandler from "virtual:vinext-rsc-entry";
import { runWithExecutionContext, type ExecutionContextLike } from "../shims/request-context.js";
import { resolveStaticAssetSignal } from "./worker-utils.js";

type WorkerAssetEnv = {
ASSETS?: {
fetch(request: Request): Promise<Response> | Response;
};
};

export default {
async fetch(request: Request, _env?: unknown, ctx?: ExecutionContextLike): Promise<Response> {
async fetch(
request: Request,
env?: WorkerAssetEnv,
ctx?: ExecutionContextLike,
): Promise<Response> {
const url = new URL(request.url);

// Normalize backslashes (browsers treat /\ as //) before any other checks.
Expand Down Expand Up @@ -52,6 +63,13 @@ export default {
const result = await (ctx ? runWithExecutionContext(ctx, handleFn) : handleFn());

if (result instanceof Response) {
if (env?.ASSETS) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean approach — app-router-entry.ts is the single resolution point for the static asset signal on Cloudflare, and the deploy-generated worker (which wraps this entry) correctly delegates without its own resolution. The test at deploy.test.ts line 396-402 explicitly guards against the double-resolution regression. Well done.

const assetResponse = await resolveStaticAssetSignal(result, {
fetchAsset: (path) =>
Promise.resolve(env.ASSETS!.fetch(new Request(new URL(path, request.url)))),
});
if (assetResponse) return assetResponse;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: double signal resolution with deploy-generated workers.

The deploy-generated worker entry (deploy.ts line ~502) calls handler.fetch(request, env, ctx) — which is this file. Since the deploy worker passes env (which has ASSETS), this block resolves the signal and returns a fully-formed asset response. Then back in the deploy worker, resolveStaticAssetSignal is called again on that response.

The second call is a no-op (header is stripped), but it still:

  • Scans headers looking for x-vinext-static-file
  • Allocates an unnecessary function call + buildHeaderRecord

More importantly, it's confusing — two layers both claim responsibility for the same signal.

Consider gating this on a condition that indicates no outer wrapper will handle it, or remove the signal handling from one of the two layers. The simplest fix: since app-router-entry.ts is the shared entry used by both direct wrangler usage and deploy-generated workers, and the deploy worker always wraps it, maybe this block should only run when there's no outer resolveStaticAssetSignal caller. One approach: only resolve here if there is no image optimization handler wrapping this (i.e., the user is using app-router-entry directly in wrangler.jsonc without a custom worker).

Alternatively, just remove the signal resolution from deploy.ts since app-router-entry.ts already handles it.

}
return result;
}

Expand Down
69 changes: 60 additions & 9 deletions packages/vinext/src/server/prod-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,11 @@ async function tryServeStatic(
compress: boolean,
cache?: StaticFileCache,
extraHeaders?: Record<string, string | string[]>,
statusCode?: number,
): Promise<boolean> {
if (pathname === "/") return false;
const responseStatus = statusCode ?? 200;
const omitBody = isNoBodyResponseStatus(responseStatus);

// ── Fast path: pre-computed headers, minimal per-request work ──
// When a cache is provided, all path validation happened at startup.
Expand All @@ -419,7 +422,11 @@ async function tryServeStatic(

// 304 Not Modified: string compare against pre-computed ETag
const ifNoneMatch = req.headers["if-none-match"];
if (typeof ifNoneMatch === "string" && matchesIfNoneMatchHeader(ifNoneMatch, entry.etag)) {
if (
responseStatus === 200 &&
typeof ifNoneMatch === "string" &&
matchesIfNoneMatchHeader(ifNoneMatch, entry.etag)
) {
if (extraHeaders) {
res.writeHead(304, { ...entry.notModifiedHeaders, ...extraHeaders });
} else {
Expand Down Expand Up @@ -449,12 +456,12 @@ async function tryServeStatic(
: entry.original;

if (extraHeaders) {
res.writeHead(200, { ...variant.headers, ...extraHeaders });
res.writeHead(responseStatus, { ...variant.headers, ...extraHeaders });
} else {
res.writeHead(200, variant.headers);
res.writeHead(responseStatus, variant.headers);
}

if (req.method === "HEAD") {
if (omitBody || req.method === "HEAD") {
res.end();
return true;
}
Expand Down Expand Up @@ -515,7 +522,11 @@ async function tryServeStatic(
// compress=false also skips all compressed variants.
// Spreading undefined is a no-op in object literals (ES2018+).
const ifNoneMatch = req.headers["if-none-match"];
if (typeof ifNoneMatch === "string" && matchesIfNoneMatchHeader(ifNoneMatch, etag)) {
if (
responseStatus === 200 &&
typeof ifNoneMatch === "string" &&
matchesIfNoneMatchHeader(ifNoneMatch, etag)
) {
const notModifiedHeaders: Record<string, string | string[]> = {
ETag: etag,
"Cache-Control": cacheControl,
Expand All @@ -539,12 +550,12 @@ async function tryServeStatic(
if (encoding) {
// Content-Length omitted intentionally: compressed size isn't known
// ahead of time, so Node.js uses chunked transfer encoding.
res.writeHead(200, {
res.writeHead(responseStatus, {
...baseHeaders,
"Content-Encoding": encoding,
Vary: "Accept-Encoding",
});
if (req.method === "HEAD") {
if (omitBody || req.method === "HEAD") {
res.end();
return true;
}
Expand All @@ -561,11 +572,11 @@ async function tryServeStatic(
}
}

res.writeHead(200, {
res.writeHead(responseStatus, {
...baseHeaders,
"Content-Length": String(resolved.size),
});
if (req.method === "HEAD") {
if (omitBody || req.method === "HEAD") {
res.end();
return true;
}
Expand Down Expand Up @@ -1044,6 +1055,46 @@ async function startAppRouterServer(options: AppRouterServerOptions) {
const request = nodeToWebRequest(req, normalizedUrl);
const response = await rscHandler(request);

const staticFileSignal = response.headers.get("x-vinext-static-file");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The x-vinext-static-file header name is used as a string literal in three separate files (app-rsc-entry.ts template, prod-server.ts, worker-utils.ts). If this protocol ever changes, it's easy to miss one. Consider extracting it into a shared constant (e.g., in worker-utils.ts) that both prod-server.ts and worker-utils.ts can import, and that the entry template can reference via JSON.stringify().

Not blocking — the current test coverage (deploy.test.ts, app-router.test.ts, serve-static.test.ts) would catch a mismatch.

if (staticFileSignal) {
let staticFilePath = "/";
try {
staticFilePath = decodeURIComponent(staticFileSignal);
} catch {
staticFilePath = staticFileSignal;
}

const staticResponseHeaders = omitHeadersCaseInsensitive(
mergeResponseHeaders({}, response),
["x-vinext-static-file", "content-encoding", "content-length", "content-type"],
);

const served = await tryServeStatic(
req,
res,
clientDir,
staticFilePath,
compress,
staticCache,
staticResponseHeaders,
response.status,
);
cancelResponseBody(response);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good: cancelResponseBody is called after tryServeStatic regardless of success. This correctly cleans up the signal response's body (which is null anyway, but defensive).

One edge case: if tryServeStatic throws (e.g., filesystem error mid-stream after headers are sent), the response body of the signal is still cancelled, but res may be in an inconsistent state. The outer try/catch at line 1100 handles this with a 500 fallback, so it should be fine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: cancelResponseBody(response) is called after tryServeStatic regardless of whether serving succeeded. Since the signal response has a null body, this is a no-op, but if the signal construction ever changes (e.g., someone adds a body for debugging), the cancel call at this position is correct and defensive. Good.

if (served) {
return;
}
await sendWebResponse(
new Response("Not Found", {
status: 404,
headers: toWebHeaders(staticResponseHeaders),
}),
req,
res,
compress,
);
return;
}

// Stream the Web Response back to the Node.js response
await sendWebResponse(response, req, res, compress);
} catch (e) {
Expand Down
48 changes: 48 additions & 0 deletions packages/vinext/src/server/worker-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ function cancelResponseBody(response: Response): void {
});
}

function buildHeaderRecord(
response: Response,
omitNames: readonly string[] = [],
): Record<string, string | string[]> {
const omitted = new Set(omitNames.map((name) => name.toLowerCase()));
const headers: Record<string, string | string[]> = {};
response.headers.forEach((value, key) => {
if (omitted.has(key.toLowerCase()) || key === "set-cookie") return;
headers[key] = value;
});
const cookies = response.headers.getSetCookie?.() ?? [];
if (cookies.length > 0) headers["set-cookie"] = cookies;
return headers;
}

export function mergeHeaders(
response: Response,
extraHeaders: Record<string, string | string[]>,
Expand Down Expand Up @@ -93,3 +108,36 @@ export function mergeHeaders(
headers: merged,
});
}

export async function resolveStaticAssetSignal(
signalResponse: Response,
options: {
fetchAsset(path: string): Promise<Response>;
},
): Promise<Response | null> {
const signal = signalResponse.headers.get("x-vinext-static-file");
if (!signal) return null;

let assetPath = "/";
try {
assetPath = decodeURIComponent(signal);
} catch {
assetPath = signal;
}

const extraHeaders = buildHeaderRecord(signalResponse, [
"x-vinext-static-file",
"content-encoding",
"content-length",
"content-type",
]);

cancelResponseBody(signalResponse);
const assetResponse = await options.fetchAsset(assetPath);
// Only preserve the middleware/status-layer override when we actually got a
// real asset response back. If the asset lookup misses (404/other non-ok),
// keep that filesystem result instead of masking it with the signal status.
const statusOverride =
assetResponse.ok && signalResponse.status !== 200 ? signalResponse.status : undefined;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent of this condition could use a comment. When assetResponse.ok is false (e.g., 404 from the asset layer), the middleware status is dropped and the asset's error status is returned directly. When the asset is found but middleware set a non-200 status (e.g., 403 via rewrite), the middleware status overrides.

This seems correct (don't override a real 404 with a middleware 403), but it's non-obvious. A one-liner would help:

Suggested change
assetResponse.ok && signalResponse.status !== 200 ? signalResponse.status : undefined;
// Only override the asset status with the middleware status when the asset was
// successfully fetched — if the asset itself errored (404, 500), preserve that.
const statusOverride =
assetResponse.ok && signalResponse.status !== 200 ? signalResponse.status : undefined;

return mergeHeaders(assetResponse, extraHeaders, statusOverride);
}
Loading
Loading