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
16 changes: 10 additions & 6 deletions packages/vinext/src/server/app-page-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,12 +435,15 @@ async function dispatchAppPageInner<TRoute extends AppPageDispatchRoute>(
styles: options.getFontStyles(),
preloads: options.getFontPreloads(),
},
revalidatedRscCapture.sideStream
? {
sideStream: revalidatedRscCapture.sideStream,
capturedRscDataRef: revalidatedCapturedRscRef,
}
: 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 ISR regeneration path now always passes a basePath key in the options object (even when it's undefined). The old code passed undefined as the 4th argument when there was no sideStream. The new shape { basePath: options.basePath, ...conditionalSpread } is cleaner — it avoids the conditional undefined 4th parameter and ensures basePath flows through ISR regeneration consistently.

basePath: options.basePath,
...(revalidatedRscCapture.sideStream
? {
sideStream: revalidatedRscCapture.sideStream,
capturedRscDataRef: revalidatedCapturedRscRef,
}
: {}),
},
);
const html = await readStreamAsText(revalidatedHtmlStream);
const rscData = await getCapturedRscDataPromise(revalidatedCapturedRscRef.value);
Expand Down Expand Up @@ -603,6 +606,7 @@ async function dispatchAppPageInner<TRoute extends AppPageDispatchRoute>(
}

return renderAppPageLifecycle({
basePath: options.basePath,
cleanPathname: options.cleanPathname,
clearRequestContext: options.clearRequestContext,
consumeDynamicUsage,
Expand Down
7 changes: 2 additions & 5 deletions packages/vinext/src/server/app-page-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { ClassificationReason } from "../build/layout-classification-types.
import { createRscRedirectLocation } from "./app-rsc-cache-busting.js";
import { mergeMiddlewareResponseHeaders } from "./middleware-response-headers.js";
import { parseNextHttpErrorDigest, parseNextRedirectDigest } from "./next-error-digest.js";
import { hasBasePath } from "../utils/base-path.js";
import { addBasePathToPathname } from "../utils/base-path.js";

export type { LayoutFlags };
export type { ClassificationReason };
Expand Down Expand Up @@ -175,10 +175,7 @@ function applyAppPageRedirectBasePath(
if (!basePath || resolved.origin !== requestOrigin) {
return resolved.toString();
}
if (hasBasePath(resolved.pathname, basePath)) {
return resolved.toString();
}
resolved.pathname = resolved.pathname === "/" ? basePath : `${basePath}${resolved.pathname}`;
resolved.pathname = addBasePathToPathname(resolved.pathname, basePath);
return resolved.toString();
}

Expand Down
2 changes: 2 additions & 0 deletions packages/vinext/src/server/app-page-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type AppPageRequestCacheLife = {
};

type RenderAppPageLifecycleOptions = {
basePath?: string;
cleanPathname: string;
clearRequestContext: () => void;
consumeDynamicUsage: () => boolean;
Expand Down Expand Up @@ -492,6 +493,7 @@ export async function renderAppPageLifecycle(
capturedRscDataRef,
fontData,
navigationContext: options.getNavigationContext(),
basePath: options.basePath,
formState: options.formState ?? null,
rscStream: rscForResponse,
scriptNonce: options.scriptNonce,
Expand Down
3 changes: 3 additions & 0 deletions packages/vinext/src/server/app-page-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export type AppPageSsrHandler = {
options?: {
formState?: ReactFormState | null;
scriptNonce?: string;
basePath?: string;
sideStream?: ReadableStream<Uint8Array>;
capturedRscDataRef?: { value: Promise<ArrayBuffer> | null };
/** When true, wait for the full React tree before emitting bytes. */
Expand All @@ -37,6 +38,7 @@ type RenderAppPageHtmlStreamOptions = {
navigationContext: unknown;
rscStream: ReadableStream<Uint8Array>;
scriptNonce?: string;
basePath?: string;
ssrHandler: AppPageSsrHandler;
/** Pre-split side stream for fused embed+capture (#981). When set,
* handleSsr skips its internal tee and accumulates raw RSC bytes. */
Expand Down Expand Up @@ -98,6 +100,7 @@ export async function renderAppPageHtmlStream(
const ssrOptions = {
formState: options.formState ?? null,
scriptNonce: options.scriptNonce,
basePath: options.basePath,
sideStream: options.sideStream,
capturedRscDataRef: options.capturedRscDataRef,
waitForAllReady: options.waitForAllReady,
Expand Down
12 changes: 10 additions & 2 deletions packages/vinext/src/server/app-ssr-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from "./html.js";
import { createRscEmbedTransform, createTickBufferedTransform } from "./app-ssr-stream.js";
import { deferUntilStreamConsumed } from "./app-page-stream.js";
import { createSsrErrorMetaRenderer } from "./app-ssr-error-meta.js";
import { AppElementsWire, type AppWireElements } from "./app-elements.js";
import { ElementsContext, Slot } from "vinext/shims/slot";
import { AppRouterContext } from "vinext/shims/internal/app-router-context";
Expand Down Expand Up @@ -169,6 +170,7 @@ export async function handleSsr(
/** Out-parameter: filled with accumulated raw RSC bytes when sideStream is consumed. */
capturedRscDataRef?: { value: Promise<ArrayBuffer> | null };
formState?: ReactFormState | null;
basePath?: string;
/** When true, wait for the full React tree (including Suspense boundaries)
* to resolve before returning the HTML stream. Used for static prerender
* and ISR cache writes to avoid caching fallback content. */
Expand Down Expand Up @@ -242,12 +244,17 @@ export async function handleSsr(
const ssrRoot = withScriptNonce(ssrTree, options?.scriptNonce);

const bootstrapScriptContent = await import.meta.viteRsc.loadBootstrapScriptContent("index");
const errorMetaRenderer = createSsrErrorMetaRenderer({
basePath: options?.basePath,
});

const htmlStream = await renderToReadableStream(ssrRoot, {
bootstrapScriptContent,
formState: options?.formState ?? null,
nonce: options?.scriptNonce,
onError(error) {
errorMetaRenderer.capture(error);

if (error && typeof error === "object" && "digest" in error) {
return String(error.digest);
}
Expand All @@ -274,14 +281,15 @@ export async function handleSsr(
let didInjectHeadHTML = false;
const getInsertedHTML = (): string => {
const insertedHTML = renderInsertedHtml(renderServerInsertedHTML());
if (didInjectHeadHTML) return insertedHTML;
const errorMetaHTML = errorMetaRenderer.flush();
if (didInjectHeadHTML) return insertedHTML + errorMetaHTML;
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 ordering here — insertedHTML + errorMetaHTML — means error meta tags appear after server-inserted HTML from useServerInsertedHTML callbacks. On the first flush (head injection), this places them after the nav scripts, form state, module preloads, and font HTML. On subsequent flushes, they appear after any new server-inserted HTML. This matches Next.js where error meta tags are appended after other server-inserted elements in the JSX array. Looks correct.


didInjectHeadHTML = true;
return buildHeadInjectionHtml(
navContext,
bootstrapScriptContent,
options?.formState ?? null,
insertedHTML,
insertedHTML + errorMetaHTML,
fontHTML,
options?.scriptNonce,
);
Expand Down
99 changes: 99 additions & 0 deletions packages/vinext/src/server/app-ssr-error-meta.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { addBasePathToPathname } from "../utils/base-path.js";
import { escapeHtmlAttr } from "./html.js";
import {
getNextErrorDigest,
parseNextHttpErrorDigest,
parseNextRedirectDigest,
} from "./next-error-digest.js";

type SsrErrorMetaRenderOptions = {
basePath?: string;
nodeEnv?: string;
};

type SsrErrorMetaRenderer = {
capture: (error: unknown) => void;
flush: () => string;
};

const PERMANENT_REDIRECT_STATUS = 308;

function prefixRedirectLocation(location: string, basePath?: string): string {
if (!basePath || !location.startsWith("/")) {
return location;
}

const hashIndex = location.indexOf("#");
const queryIndex = location.indexOf("?");
const pathnameEnd =
queryIndex === -1
? hashIndex === -1
? location.length
: hashIndex
: hashIndex === -1
? queryIndex
: Math.min(queryIndex, hashIndex);
const pathname = location.slice(0, pathnameEnd);

return addBasePathToPathname(pathname, basePath) + location.slice(pathnameEnd);
}

function renderSsrErrorMetaTag(error: unknown, options: SsrErrorMetaRenderOptions): string {
const digest = getNextErrorDigest(error);
if (!digest) return "";

const httpError = parseNextHttpErrorDigest(digest);
if (httpError) {
let html = '<meta name="robots" content="noindex" />';
if ((options.nodeEnv ?? process.env.NODE_ENV) === "development") {
html += '<meta name="next-error" content="not-found" />';
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 next-error dev meta tag is emitted for all HTTP error fallback statuses (403, 401, etc.), not just 404. The content="not-found" value is a bit misleading for a 403 forbidden error. Next.js does the same thing (isHTTPAccessFallbackError covers all of these and emits content="not-found" for all of them), so this is correct for parity — but worth noting the Next.js oddity if someone wonders about it later.

}
return html;
}

const redirect = parseNextRedirectDigest(digest);
if (!redirect) return "";

const delay = redirect.status === PERMANENT_REDIRECT_STATUS ? 0 : 1;
const location = prefixRedirectLocation(redirect.url, options.basePath);
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 observation: Next.js's addPathPrefix in the streaming meta tag path works on the raw URL string from getURLFromRedirectError(), which is already decoded. The prefixRedirectLocation here receives the same decoded URL from parseNextRedirectDigest. The manual pathname extraction (lines 26-36) is necessary here since new URL() would choke on relative paths. This is correct — just noting the intentional divergence from applyAppPageRedirectBasePath which can afford to use new URL(location, requestUrl) because it has a request URL for resolution.

return (
'<meta id="__next-page-redirect" http-equiv="refresh" content="' +
delay +
";url=" +
escapeHtmlAttr(location) +
'" />'
);
}

export function renderSsrErrorMetaTags(
errors: readonly unknown[],
options: SsrErrorMetaRenderOptions = {},
): string {
let html = "";

for (const error of errors) {
html += renderSsrErrorMetaTag(error, options);
}

return html;
}

export function createSsrErrorMetaRenderer(
options: SsrErrorMetaRenderOptions = {},
): SsrErrorMetaRenderer {
const capturedErrors: unknown[] = [];
let flushedUntil = 0;

return {
capture(error) {
capturedErrors.push(error);
},
flush() {
if (flushedUntil >= capturedErrors.length) return "";

const html = renderSsrErrorMetaTags(capturedErrors.slice(flushedUntil), options);
flushedUntil = capturedErrors.length;
return html;
},
};
}
6 changes: 5 additions & 1 deletion packages/vinext/src/server/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ export function safeJsonStringify(data: unknown): string {
}

export function escapeHtmlAttr(value: string): string {
return value.replace(/&/g, "&amp;").replace(/"/g, "&quot;");
return value
.replace(/&/g, "&amp;")
.replace(/"/g, "&quot;")
.replace(/</g, "&lt;")
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 addition — < and > escaping makes escapeHtmlAttr properly comprehensive for any HTML attribute context. The existing callers (font URLs, nonces, module preload HREFs) won't be affected since those values don't contain angle brackets, but it's correct defense-in-depth for the new redirect URL use case.

.replace(/>/g, "&gt;");
}

export function createNonceAttribute(nonce?: string): string {
Expand Down
10 changes: 10 additions & 0 deletions packages/vinext/src/utils/base-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@ export function stripBasePath(pathname: string, basePath: string): string {
return pathname.slice(basePath.length) || "/";
}

/**
* Add the configured basePath to a pathname unless it is already inside that
* basePath. Query strings and hashes must be handled by callers before calling
* this pathname-only helper.
*/
export function addBasePathToPathname(pathname: string, basePath: string | undefined): string {
if (!basePath || hasBasePath(pathname, basePath)) return pathname;
return pathname === "/" ? basePath : `${basePath}${pathname}`;
}

/**
* Remove trailing slashes from a pathname while preserving the root "/".
* Collapses any number of trailing slashes ("/a//" → "/a"). Used by the
Expand Down
32 changes: 32 additions & 0 deletions tests/app-page-execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,38 @@ describe("app page execution helpers", () => {

expect(alreadyPrefixed.headers.get("location")).toBe("https://example.com/blog/about");

const alreadyPrefixedRootWithQuery = await buildAppPageSpecialErrorResponse({
basePath: "/blog",
clearRequestContext,
isRscRequest: false,
request: new Request("https://example.com/blog/protected"),
specialError: {
kind: "redirect",
location: "/blog?from=checkout",
statusCode: 307,
},
});

expect(alreadyPrefixedRootWithQuery.headers.get("location")).toBe(
"https://example.com/blog?from=checkout",
);

const alreadyPrefixedRootWithHash = await buildAppPageSpecialErrorResponse({
basePath: "/blog",
clearRequestContext,
isRscRequest: false,
request: new Request("https://example.com/blog/protected"),
specialError: {
kind: "redirect",
location: "/blog#top",
statusCode: 307,
},
});

expect(alreadyPrefixedRootWithHash.headers.get("location")).toBe(
"https://example.com/blog#top",
);

// No basePath configured → behavior unchanged (resolves against the
// request URL as before).
const unconfigured = await buildAppPageSpecialErrorResponse({
Expand Down
24 changes: 24 additions & 0 deletions tests/app-page-stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,30 @@ describe("app page stream helpers", () => {
);
});

it("forwards basePath to the SSR handler", async () => {
const ssrHandler = vi.fn(async () => createStream(["<html>base-path</html>"]));

const htmlStream = await renderAppPageHtmlStream({
basePath: "/docs",
fontData: createAppPageFontData({
getLinks: () => [],
getPreloads: () => [],
getStyles: () => [],
}),
navigationContext: null,
rscStream: createStream(["flight"]),
ssrHandler: { handleSsr: ssrHandler },
});

await expect(new Response(htmlStream).text()).resolves.toBe("<html>base-path</html>");
expect(ssrHandler).toHaveBeenCalledWith(
expect.anything(),
null,
expect.anything(),
expect.objectContaining({ basePath: "/docs" }),
);
});

it("defers clearRequestContext until the HTML stream body is fully consumed", async () => {
// Regression test for issue #660: clearRequestContext() must not race the
// lazy RSC/SSR stream pipeline. It should be called only after the HTTP
Expand Down
Loading
Loading