Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
2a9a102
fix(plugin): resolve vinext/shims/* package subpaths to local shim files
NathanDrake2406 Jun 12, 2026
99789d7
fix(app-router): recover SSR shell render errors via __next_error__ d…
NathanDrake2406 Jun 12, 2026
dab9c72
refactor(review): address PR 1908 non-blocking notes
NathanDrake2406 Jun 12, 2026
7f8b8ba
refactor(review): clarify shell recovery assumptions
james-elicx Jun 12, 2026
f8ff07c
fix(ssr): cancel abandoned prerender streams
james-elicx Jun 12, 2026
ad6daac
refactor(ssr): clarify error shell root options
james-elicx Jun 12, 2026
2a6292a
fix(isr): recover shell errors during regeneration
james-elicx Jun 12, 2026
57b01b4
fix(ssr): scope client recovery to marked shells
james-elicx Jun 12, 2026
2da1242
fix(plugin): explicitly filter vinext shim subpaths
james-elicx Jun 12, 2026
e9f0119
fix(plugin): retain null-prefixed shim resolution
james-elicx Jun 12, 2026
a2f481b
test(nextjs-compat): cover no-boundary shell recovery fallback
NathanDrake2406 Jun 12, 2026
c760b2a
Merge upstream/main into nathan/next-dynamic-css
NathanDrake2406 Jun 13, 2026
001f36a
fix(app-router): preserve shell recovery error semantics
james-elicx Jun 13, 2026
8a259f4
test(app-router): harden shell recovery cache semantics
james-elicx Jun 13, 2026
fe3df57
fix(app-router): mark global error responses uncacheable
james-elicx Jun 13, 2026
e58a313
merge: sync main into next dynamic css
james-elicx Jun 13, 2026
98b93df
test(app-router): align error response status expectations
james-elicx Jun 13, 2026
53aef71
merge: sync main into next dynamic css
james-elicx Jun 14, 2026
384c40e
fix(build): preserve null-prefixed og resolution
james-elicx Jun 14, 2026
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
1 change: 1 addition & 0 deletions packages/vinext/src/entries/app-rsc-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,7 @@ export default __createAppRscHandler({
getSourceRoute(sourceRouteIndex) {
return routes[sourceRouteIndex];
},
hasCustomGlobalError: ${globalErrorVar ? `Boolean(${globalErrorVar}?.default)` : "false"},

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.

Boolean(${globalErrorVar}?.default) correctly treats a global-error file with no default export as "no custom global error" — so an app with a malformed global-error.tsx (file present, no default) would still get the __next_error__ recovery shell rather than the server-side boundary path. That's a reasonable choice, but worth confirming it matches Next.js, which I believe validates/throws on a global-error without a default export rather than silently falling back. Non-blocking, but a candidate for a follow-up parity check.

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.

Boolean(${globalErrorVar}?.default) treats a global-error.tsx that exists but has no default export as "no custom global error", so a malformed global-error file would silently get the __next_error__ recovery shell instead of erroring. That's a reasonable default, but Next.js validates app-router special files and throws on a missing default export rather than silently falling back. Non-blocking — worth a follow-up parity check (also flagged in a prior round).

hasGenerateStaticParams: __generateStaticParams.length > 0,
hasPageDefaultExport: !!PageComponent,
hasPageModule: !!route.page,
Expand Down
21 changes: 19 additions & 2 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ import { createRequire } from "node:module";
import fs from "node:fs";
import { randomBytes, randomUUID } from "node:crypto";
import commonjs from "vite-plugin-commonjs";
import { normalizePathSeparators, stripViteModuleQuery } from "./utils/path.js";
import { normalizePathSeparators, stripJsExtension, stripViteModuleQuery } from "./utils/path.js";

// Install the process-level peer-disconnect backstop at module load.
// Vite plugin lifecycle hooks (config / configureServer) proved
Expand Down Expand Up @@ -2578,6 +2578,23 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
);
}

// Runtime helper modules embedded into generated entries import
// vinext's own shims by package subpath (for example
// `vinext/shims/headers`). Source checkouts also alias userland
// `next/*` imports to the local shim files. Resolve both forms
// through this plugin so request-scoped singleton state is not split
// between source shims and the package export copy. These internal
// package subpaths deliberately resolve to the plain shim today; if
// one gains an environment-specific variant, handle it here before
// returning rather than relying on the userland `next/*` map below.
const vinextShimPrefix = "vinext/shims/";
if (cleanId.startsWith(vinextShimPrefix)) {

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 branch now intercepts every vinext/shims/* subpath before the _reactServerShims check below. That's correct for the current shim set (the react-server variants are keyed on userland next/* specifiers like next/navigation, not vinext/shims/navigation), so internal vinext/shims/navigation imports have always resolved to the plain shim. But if a future shim needs a .react-server variant when imported via its vinext/shims/* package subpath, this early return would silently shadow it. A short comment noting that this branch deliberately does not handle per-environment variants (and where to add that if needed) would prevent a subtle future regression.

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 new vinext/shims/ branch is reachable only because the resolveId filter regex (/(?:next\/|…)/ at index.ts:2507) happens to match vinext/shims/*vinext/ contains the substring next/. This is load-bearing but entirely implicit: if the filter is ever tightened to anchor next/ (e.g. ^next\/ or a word boundary), this branch silently stops firing and the singleton-state split this PR fixes would quietly regress, with no test catching it (the new shims test calls the handler directly, bypassing the filter).

Consider either (a) adding an explicit vinext\/shims\/ alternative to the filter regex so the match is intentional, or (b) a one-line comment here noting the branch depends on the incidental next/-substring match in the filter above.

return resolveShimModulePath(
_shimsDir,
stripJsExtension(cleanId.slice(vinextShimPrefix.length)),

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 robustness: cleanId here is only \0-stripped, not query-stripped (unlike line 2514's derivation, which also leaves the query). stripJsExtension won't strip a trailing query, so a specifier like vinext/shims/navigation?v=123 would resolve to a wrong base name. Internal package-subpath imports never carry a query, so this isn't reachable today — but if you want it to be defensive, run the subpath through stripViteModuleQuery before stripJsExtension.

);
}

// Shims with react-server variants — resolve per-environment.
// These are NOT in resolve.alias (Vite's alias plugin runs
// before enforce:"pre" plugins and can't be overridden).
Expand All @@ -2588,7 +2605,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
this.environment?.name === "rsc"
? `${reactServerShim}.react-server`
: reactServerShim;
return resolveShimModulePath(_shimsDir, shimName);
return resolveShimModulePath(_shimsDir, stripJsExtension(shimName));

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: stripJsExtension(shimName) is a no-op here. _reactServerShims values are bare base names ("navigation", see index.ts:646-650) and ${reactServerShim}.react-server likewise never carries .js — only the map key (the userland specifier) can be extensionful, and that's already normalized via cleanId before the .get(cleanId) lookup. The new vinext/shims/* branch at line 2594 is where stripJsExtension actually matters (the subpath can be extensionful). Consider dropping it here so it doesn't imply the value side needs normalization:

Suggested change
return resolveShimModulePath(_shimsDir, stripJsExtension(shimName));
return resolveShimModulePath(_shimsDir, shimName);

}
},
},
Expand Down
43 changes: 33 additions & 10 deletions packages/vinext/src/server/app-browser-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
setServerCallback,
} from "@vitejs/plugin-rsc/browser";
import { flushSync } from "react-dom";
import { hydrateRoot } from "react-dom/client";
import { createRoot, hydrateRoot } from "react-dom/client";
import "../client/instrumentation-client.js";
import { notifyAppRouterTransitionStart } from "../client/instrumentation-client-state.js";
import {
Expand Down Expand Up @@ -1603,16 +1603,34 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
onCaughtError: prodOnCaughtError,
onUncaughtError,
});
window.__VINEXT_RSC_ROOT__ = hydrateRootInTransition({
children: createElement(BrowserRoot, {
initialElements: root,
initialNavigationSnapshot,
}),
container: document,
hydrateRoot,
options: hydrateRootOptions,
startTransition,
const children = createElement(BrowserRoot, {
initialElements: root,
initialNavigationSnapshot,
});
// The `__next_error__` marker is intentionally broad: it detects the default
// error document generally, not only the new SSR shell-error recovery path.
// Both `DefaultGlobalError` (used here) and the boundary renderer emit
// `<html id="__next_error__">`, so this branch also fires for legacy server-
// rendered global-error pages. That is upstream-compatible — the browser
// should always createRoot when the document is the default error shell.
if (document.documentElement.id === "__next_error__") {

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.

Behavior-change worth confirming: on main, bootstrapHydration always used hydrateRootInTransition (hydration). This branch now switches to createRoot (full client re-render, no hydration) for any document whose documentElement.id === "__next_error__", which — as your comment notes — also includes the pre-existing server-rendered default-error page (digest error, no custom global-error.tsx). That path previously hydrated the server markup; it now discards and re-renders client-side.

This is plausibly upstream-compatible (Next.js client-renders the recovery shell), and the embedded flight payload encodes the same error tree so the rendered content matches. But the new browser spec only exercises the SSR-only-throw recovery path, not the legacy server-digest-error path through this same branch. Worth a quick confirmation (or a test) that a genuine server-side digest error with no custom global-error still renders the default error card correctly via createRoot rather than relying on the now-removed hydration path.

for (const style of document.querySelectorAll("style[data-vinext-error-shell-style]")) {
style.remove();
}
startTransition(() => {
const clientRoot = createRoot(document, hydrateRootOptions);

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.

hydrateRootOptions is built by createVinextHydrateRootOptions, which always includes formState (a hydrateRoot-only option). createRoot's RootOptions does not have formState — it's silently ignored at runtime, so this isn't a bug, but it's a semantic mismatch worth noting: in the recovery path there is no server-rendered form to hydrate, so the captured formState is meaningless here. The structural cast works only because HydrateRootOptions is a superset React tolerates. A short comment (e.g. "formState is inert for createRoot; we reuse the hydrate options object only for the error handlers") would prevent a future reader from assuming form-state replay happens on the recovery path.

clientRoot.render(children);
window.__VINEXT_RSC_ROOT__ = clientRoot;
});
} else {
window.__VINEXT_RSC_ROOT__ = hydrateRootInTransition({
children,
container: document,
hydrateRoot,
options: hydrateRootOptions,
startTransition,
});
}

const navigateRsc: NavigationRuntimeNavigate = async function navigateRsc(
href: string,
Expand Down Expand Up @@ -2190,6 +2208,11 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
// original document from there, so let the next HMR update reload the
// current URL. If the edit fixed the error the page comes back clean; if
// not, initial dev server errors re-populate the overlay.
//
// The `__next_error__` marker is broad — see the matching comment in
// bootstrapHydration above. Reloading here is safe for any default-error
// document because the dev server will render the current state of the
// source after the edit.
if (document.documentElement.id === "__next_error__") {
window.location.reload();
return;
Expand Down
6 changes: 6 additions & 0 deletions packages/vinext/src/server/app-page-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,11 @@ type DispatchAppPageOptions<TRoute extends AppPageDispatchRoute> = {
probeLayoutAt: (layoutIndex: number, layoutParamAccess?: AppLayoutParamAccessTracker) => unknown;
probePage: () => unknown;
expireSeconds?: number;
/** True when the app supplies a custom global-error.tsx (generated entry
* knows this at build time). Gates SSR shell-error recovery: without a
* custom global-error, shell errors serve the default `__next_error__`
* recovery document instead of a server-side boundary re-render. */
hasCustomGlobalError?: boolean;
renderErrorBoundaryPage: (error: unknown) => Promise<Response | null>;
renderHttpAccessFallbackPage: (
statusCode: number,
Expand Down Expand Up @@ -979,6 +984,7 @@ async function dispatchAppPageInner<TRoute extends AppPageDispatchRoute>(
return renderPageSpecialError(options, specialError);
},
renderToReadableStream: options.renderToReadableStream,
hasCustomGlobalError: options.hasCustomGlobalError,

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 threads the flag into the initial render lifecycle. The background-revalidation handleSsr call in this same file (~line 653, revalidatedSsrEntry.handleSsr(...) with waitForAllReady: true at line 666) does not get the equivalent fallbackToErrorDocumentOnShellError, so revalidation of a throw-to-opt-out route permanently rejects after the first render caches a 200 recovery shell. See the review summary for details — worth threading the flag there too or documenting the divergence.

routePattern: route.pattern,
runWithSuppressedHookWarning(probe) {
return options.runWithSuppressedHookWarning(probe);
Expand Down
5 changes: 5 additions & 0 deletions packages/vinext/src/server/app-page-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ type RenderAppPageLifecycleOptions = {
element: ReactNode | AppOutgoingElements,
options: { onError: AppPageBoundaryOnError },
) => ReadableStream<Uint8Array>;
/** True when the app supplies a custom global-error.tsx. When false, SSR
* shell render errors fall back to the default `__next_error__` recovery
* document instead of a server-side boundary re-render. */
hasCustomGlobalError?: boolean;
routePattern: string;
runWithSuppressedHookWarning<T>(probe: () => Promise<T>): Promise<T>;
scriptNonce?: string;
Expand Down Expand Up @@ -836,6 +840,7 @@ export async function renderAppPageLifecycle(
rscStream: rscForResponse,
scriptNonce: options.scriptNonce,
sideStream: rscCapture.sideStream,
hasCustomGlobalError: options.hasCustomGlobalError,
ssrHandler,
waitForAllReady: options.isPrerender,
});
Expand Down
11 changes: 11 additions & 0 deletions packages/vinext/src/server/app-page-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ export type AppPageSsrHandler = {
waitForAllReady?: boolean;
/** Dev-only: original server error to surface in the browser overlay. */
initialDevServerError?: unknown;
/** When true, an SSR-phase-only shell render error resolves to the
* default `__next_error__` error-document shell (with the original
* flight payload and bootstrap) instead of rejecting. See handleSsr. */
fallbackToErrorDocumentOnShellError?: boolean;
},
) => Promise<ReadableStream<Uint8Array> | AppSsrRenderResult>;
};
Expand Down Expand Up @@ -166,6 +170,10 @@ type RenderAppPageHtmlStreamOptions = {
waitForAllReady?: boolean;
/** Dev-only: original server error to surface in the browser overlay. */
initialDevServerError?: unknown;
/** True when the app supplies a custom global-error.tsx. Disables the
* default error-document shell fallback so SSR shell errors keep driving
* the server-rendered global-error boundary re-render. */
hasCustomGlobalError?: boolean;
};

type RenderAppPageHtmlResponseOptions = {
Expand Down Expand Up @@ -227,6 +235,9 @@ export async function renderAppPageHtmlStream(
capturedRscDataRef: options.capturedRscDataRef,
waitForAllReady: options.waitForAllReady,
initialDevServerError: options.initialDevServerError,
// Only when the caller affirmatively knows there is no custom
// global-error.tsx; undefined (unknown) keeps reject semantics.
fallbackToErrorDocumentOnShellError: options.hasCustomGlobalError === false,
};

const rawResult = await options.ssrHandler.handleSsr(
Expand Down
93 changes: 89 additions & 4 deletions packages/vinext/src/server/app-ssr-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import { BfcacheStateKeyMapContext, ElementsContext, Slot } from "vinext/shims/s
import { AppRouterContext } from "vinext/shims/internal/app-router-context";
import { createClientReferencePreloader } from "./app-client-reference-preloader.js";
import { RSC_FORM_STATE_GLOBAL } from "./app-browser-hydration.js";
import DefaultGlobalError from "vinext/shims/default-global-error";

/**
* `@types/react-dom` does not yet type `maxHeadersLength` (it pairs with the
Expand Down Expand Up @@ -106,6 +107,58 @@ function getErrorMessage(error: unknown): string {
return Object.prototype.toString.call(error);
}

function createUtf8Stream(html: string): ReadableStream<Uint8Array> {
const encoder = new TextEncoder();
return new ReadableStream<Uint8Array>({
start(controller) {
controller.enqueue(encoder.encode(html));
controller.close();
},
});
}

function buildBootstrapModuleScript(bootstrapModuleUrl?: string, nonce?: string): string {
if (!bootstrapModuleUrl) return "";
return (
`<script type="module"${createNonceAttribute(nonce)} src="` +
escapeHtmlAttr(bootstrapModuleUrl) +
'" id="_R_" async=""></script>'
);
}

function markErrorShellStyle(html: string): string {
// DefaultGlobalError intentionally emits one style tag. If that changes,
// update this marker logic without touching styles inserted later by the
// stream transforms.
return html.replace("<style>", '<style data-vinext-error-shell-style="">');

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.

markErrorShellStyle replaces only the first <style>. That's correct today because DefaultGlobalError renders exactly one <style dangerouslySetInnerHTML>, and any inline-CSS/server-inserted styles are added later by the transform (so they correctly stay unmarked and are preserved by the browser cleanup). This is fragile against future edits to DefaultGlobalError adding a second <style> — worth a one-line comment noting the single-style assumption, since the browser-side style[data-vinext-error-shell-style] removal depends on it.

}

function renderSsrErrorDocumentShell(
bootstrapModuleUrl?: string,
nonce?: string,
): ReadableStream<Uint8Array> {
const html = markErrorShellStyle(
renderToStaticMarkup(
createReactElement(DefaultGlobalError, {
error: null,
}),
),
);
const bootstrapScript = buildBootstrapModuleScript(bootstrapModuleUrl, nonce);
if (!bootstrapScript) {
return createUtf8Stream(`<!DOCTYPE html>${html}`);
}

const documentClose = "</body></html>";
if (!html.endsWith(documentClose)) {
return createUtf8Stream(`<!DOCTYPE html>${html}${bootstrapScript}`);
}

return createUtf8Stream(
`<!DOCTYPE html>${html.slice(0, -documentClose.length)}${bootstrapScript}${documentClose}`,
);
}

function renderInsertedHtml(insertedElements: readonly unknown[]): string {
let insertedHTML = "";

Expand Down Expand Up @@ -306,6 +359,12 @@ export async function handleSsr(
* to resolve before returning the HTML stream. Used for static prerender
* and ISR cache writes to avoid caching fallback content. */
waitForAllReady?: boolean;
/** When true, an SSR-phase shell render error (no RSC-originated `digest`)
* resolves to the default `__next_error__` error-document shell with the
* original flight payload and bootstrap module, instead of rejecting.
* Callers set this only when the app has no custom global-error.tsx, so
* boundary re-render semantics are unaffected. */
fallbackToErrorDocumentOnShellError?: boolean;
},
): Promise<AppSsrRenderResult> {
return runWithNavigationContext(async () => {
Expand Down Expand Up @@ -501,8 +560,6 @@ export async function handleSsr(
},
};

const htmlStream = await renderToReadableStream(ssrRoot, ssrRenderOptions);

// Populated before any SSR request runs: at prod-server startup
// (prod-server.ts) or via build-time bundle injection (index.ts). Left
// undefined in dev, which naturally disables inline CSS there.
Expand Down Expand Up @@ -560,8 +617,36 @@ export async function handleSsr(
const getBeforeInteractiveHeadHTML = (): string =>
renderBeforeInteractiveInlineScripts(beforeInteractiveInlineScripts);

if (options?.waitForAllReady === true) {
await htmlStream.allReady;
let renderedHtmlStream: Awaited<ReturnType<typeof renderToReadableStream>> | undefined;
let htmlStream: ReadableStream<Uint8Array>;
try {
renderedHtmlStream = await renderToReadableStream(ssrRoot, ssrRenderOptions);
if (options?.waitForAllReady === true) {
await renderedHtmlStream.allReady;
}
htmlStream = renderedHtmlStream;

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.

On the waitForAllReady (prerender) path, if renderedHtmlStream.allReady rejects, the already-created renderedHtmlStream is abandoned without being cancelled before we fall through to renderSsrErrorDocumentShell. Not a correctness bug, but it leaves an un-consumed/un-cancelled React stream behind. Consider void renderedHtmlStream.cancel().catch(() => {}) in the catch before building the error shell.

} catch (error) {
void renderedHtmlStream?.cancel().catch(() => {});
// Contract with renderAppPageHtmlStreamWithRecovery (app-page-stream.ts):
// a rejected handleSsr drives redirect()/notFound() responses and
// local/global error.tsx boundary re-renders. Errors that originate in
// the RSC render (rethrown here through the flight deserializer) carry
// a string `digest` and must always propagate so those semantics stay
// intact. Only an SSR-phase-only shell error — and only when the app
// has no custom global-error.tsx (the caller sets the flag) — falls
// back to the default `__next_error__` document shell: the original
// flight payload plus the bootstrap module are still emitted, so the
// browser recovers by re-rendering the real tree from the embedded
// RSC data (Next.js shell-error recovery semantics). This resolves
// as a successful shell render, so the caller preserves its normal
// status and ISR eligibility rather than treating it as a 500.
if (
options?.fallbackToErrorDocumentOnShellError !== true ||
typeof (error as { digest?: unknown } | null)?.digest === "string"
) {
throw error;
}
htmlStream = renderSsrErrorDocumentShell(bootstrapModuleUrl, options?.scriptNonce);
}

const finalStream = deferUntilStreamConsumed(
Expand Down
9 changes: 9 additions & 0 deletions packages/vinext/src/utils/path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,12 @@ export function stripViteModuleQuery(id: string): string {
const queryIndex = id.search(/[?#]/);
return queryIndex === -1 ? id : id.slice(0, queryIndex);
}

/** Strip a trailing `.js` extension from a module specifier so
* `resolveShimModulePath` looks for the correct base name (e.g. `headers.js`
* → `headers`). Public and internal shim imports may carry extensionful
* subpaths; normalising before resolution prevents looking for non-existent
* files like `headers.js.ts`. */
export function stripJsExtension(name: string): string {
return name.endsWith(".js") ? name.slice(0, -3) : name;
}
Loading
Loading