Skip to content

perf: eliminate page probe double-execution of page components #404

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.

Sign up for GitHub

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

Draft
james-elicx wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

perf: eliminate page probe double-execution of page components #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
130 changes: 88 additions & 42 deletions packages/vinext/src/entries/app-rsc-entry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2177,63 +2177,109 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
if (_layoutProbeResult instanceof Response) return _layoutProbeResult;
}

// Pre-render the page component to catch redirect()/notFound() thrown synchronously.
// Server Components are just functions — we can call PageComponent directly to detect
// these special throws before starting the RSC stream.
//
// For routes with a loading.tsx Suspense boundary, we skip awaiting async components.
// The Suspense boundary + rscOnError will handle redirect/notFound thrown during
// streaming, and blocking here would defeat streaming (the slow component's delay
// would be hit before the RSC stream even starts).
//
// Because this calls the component outside React's render cycle, hooks like use()
// trigger "Invalid hook call" console.error in dev. The module-level ALS patch
// suppresses the warning only within this request's execution context.
const _hasLoadingBoundary = !!(route.loading && route.loading.default);
const _pageProbeResult = await _suppressHookWarningAls.run(true, async () => {
try {
const testResult = PageComponent({ params });
// If it's a promise (async component), only await if there's no loading boundary.
// With a loading boundary, the Suspense streaming pipeline handles async resolution
// and any redirect/notFound errors via rscOnError.
if (testResult && typeof testResult === "object" && typeof testResult.then === "function") {
if (!_hasLoadingBoundary) {
await testResult;
} else {
// Suppress unhandled promise rejection — with a loading boundary,
// redirect/notFound errors are handled by rscOnError during streaming.
testResult.catch(() => {});
}
}
} catch (preRenderErr) {
const specialResponse = await handleRenderError(preRenderErr);
if (specialResponse) return specialResponse;
// Non-special errors from the pre-render test are expected (e.g. use() hook
// fails outside React's render cycle, client references can't execute on server).
// Only redirect/notFound/forbidden/unauthorized are actionable here — other
// errors will be properly caught during actual RSC/SSR rendering below.
}
return null;
});
if (_pageProbeResult instanceof Response) return _pageProbeResult;

// Mark end of compile phase: route matching, middleware, tree building are done.
if (process.env.NODE_ENV !== "production") __compileEnd = performance.now();

// Render to RSC stream.
// Track non-navigation RSC errors so we can detect when the in-tree global
// ErrorBoundary catches during SSR (producing double <html>/<body>) and
// re-render with renderErrorBoundaryPage (which skips layouts for global-error).
//
// Also track the first redirect/notFound/forbidden/unauthorized error thrown
// during rendering. Previously, a "page probe" called the page component
// outside React's render cycle to detect these throws before streaming.
// That caused double data fetching and ~2x CPU time. Instead, we now
// buffer the RSC stream and check onError — components execute only once
// during the actual RSC render.
let _rscErrorForRerender = null;
let _caughtSpecialError = null;
const _baseOnError = createRscOnErrorHandler(request, cleanPathname, route.pattern);
const onRenderError = function(error, requestInfo, errorContext) {
// Capture the first redirect/notFound/forbidden/unauthorized for
// stream-buffered detection (replaces the old page probe).
if (!_caughtSpecialError && error && typeof error === "object" && "digest" in error) {
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 10, 2026

Choose a reason for hiding this comment

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

The !_caughtSpecialError guard means only the first special error is captured. In the buffered-stream path, if the page throws notFound() but a sibling component in the same render tree throws redirect(), whichever fires onError first wins. This is probably fine since redirect should take priority in most real scenarios, and layout-level errors are already caught by the layout probe above. Worth a brief comment noting the first-wins semantics are intentional.

const digest = String(error.digest);
if (
digest.startsWith("NEXT_REDIRECT;") ||
digest === "NEXT_NOT_FOUND" ||
digest.startsWith("NEXT_HTTP_ERROR_FALLBACK;")
) {
_caughtSpecialError = error;
}
}
if (!(error && typeof error === "object" && "digest" in error)) {
_rscErrorForRerender = error;
}
return _baseOnError(error, requestInfo, errorContext);
};
const rscStream = renderToReadableStream(element, { onError: onRenderError });

// Detect page-level redirect/notFound/forbidden/unauthorized errors before
// committing to the HTTP response status code. Strategy varies by page type:
//
// 1. Routes WITH loading.tsx (Suspense): stream directly. Errors inside the
// Suspense boundary flow through the RSC stream for client-side handling.
//
// 2. SYNC pages (no loading.tsx): a lightweight sync-only probe catches
// redirect/notFound thrown synchronously by the page function. This is
// cheap — sync server components do no data fetching. The probe result
// is discarded; only the thrown error matters.
//
// 3. ASYNC pages (no loading.tsx): buffer the entire RSC stream to catch
// redirect/notFound thrown after await (e.g. fetch then notFound).
// This is the key optimization — the old "page probe" awaited the full
// async component, causing every data fetch to run twice. Buffering the
// stream instead means components execute only once. Latency is the same
// because without Suspense, nothing can stream until all async work is done.
const _hasLoadingBoundary = !!(route.loading && route.loading.default);
const _isAsyncPage = PageComponent.constructor?.name === "AsyncFunction";
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 10, 2026

Choose a reason for hiding this comment

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

This is the crux of the change. PageComponent.constructor?.name checks the constructor of the function object itself, not the return value. For native async function declarations this works, but:

  • If esbuild/Vite downlevels async to a generator wrapper (unlikely with modern targets, but possible with target: "es2015"), the constructor would be Function, not AsyncFunction.
  • If the page is wrapped by an HOC (withAuth(Page)), the wrapper is what's checked, not the inner async function.
  • Minifiers can mangle .name in production builds.

The consequence of a false negative (async page classified as sync) is that the sync probe runs, catches nothing (async throws happen after the sync probe returns), and the buffering path is skipped — so redirect()/notFound() after an await would not produce a proper 302/404.

Have you verified this works with actual Vite RSC build output for both dev and production? Consider whether it's safer to make stream-buffering the default for non-Suspense pages, with the sync detection as the optimization.

let _bufferedRscStream;

if (_hasLoadingBoundary) {
// Suspense route: stream directly. Errors inside the Suspense boundary
// (loading.tsx) flow through the RSC stream as error references.
_bufferedRscStream = rscStream;
} else if (!_isAsyncPage) {
// Sync page: lightweight probe catches sync redirect/notFound/forbidden/
// unauthorized from the page function itself. Errors from async children
// inside inline <Suspense> boundaries flow through the stream as before.
const _syncProbeResult = await _suppressHookWarningAls.run(true, async () => {
try {
PageComponent({ params });
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 10, 2026

Choose a reason for hiding this comment

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

Pre-existing behavior, but worth noting: the sync probe still calls PageComponent({ params }) without searchParams, while the actual RSC render (via buildPageElement) passes both params and searchParams. If the sync page component accesses props.searchParams synchronously and throws based on its value, the probe would see undefined instead of the real searchParams. This is unlikely to matter in practice but is a subtle divergence from the actual render path.

} catch (syncErr) {
const specialResponse = await handleRenderError(syncErr);
if (specialResponse) return specialResponse;
}
return null;
});
if (_syncProbeResult instanceof Response) return _syncProbeResult;
_bufferedRscStream = rscStream;
} else {
// Async page without Suspense: buffer the entire RSC stream to catch
// redirect/notFound thrown after await. Components execute only once
// (during RSC rendering), eliminating the double data fetching that
// the old page probe caused.
const _rscReader = rscStream.getReader();
Copy link
Contributor

@ask-bonk ask-bonk bot Mar 10, 2026

Choose a reason for hiding this comment

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

The buffering approach is clean. One edge case to consider: if renderToReadableStream errors before producing any chunks (e.g., the element tree itself is invalid), _caughtSpecialError would be null and _chunks would be empty. The code handles this correctly — it creates an empty buffered stream and rendering continues to the SSR phase where the error would surface. Just confirming this is intentional.

const _chunks = [];
while (true) {
const { value, done } = await _rscReader.read();
if (value) _chunks.push(value);
if (done) break;
}

if (_caughtSpecialError) {
const specialResponse = await handleRenderError(_caughtSpecialError);
if (specialResponse) return specialResponse;
}

_bufferedRscStream = new ReadableStream({
start(controller) {
for (let _ci = 0; _ci < _chunks.length; _ci++) controller.enqueue(_chunks[_ci]);
controller.close();
},
});
}

if (isRscRequest) {
// Direct RSC stream response (for client-side navigation)
// NOTE: Do NOT clear headers/navigation context here!
Expand Down Expand Up @@ -2299,7 +2345,7 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
const compileMs = __compileEnd !== undefined ? Math.round(__compileEnd - __reqStart) : -1;
responseHeaders["x-vinext-timing"] = handlerStart + "," + compileMs + ",-1";
}
return new Response(rscStream, { status: _mwCtx.status || 200, headers: responseHeaders });
return new Response(_bufferedRscStream, { status: _mwCtx.status || 200, headers: responseHeaders });
}

// Collect font data from RSC environment before passing to SSR
Expand All @@ -2324,7 +2370,7 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
let htmlStream;
try {
const ssrEntry = await import.meta.viteRsc.loadModule("ssr", "index");
htmlStream = await ssrEntry.handleSsr(rscStream, _getNavigationContext(), fontData);
htmlStream = await ssrEntry.handleSsr(_bufferedRscStream, _getNavigationContext(), fontData);
// Shell render complete; Suspense boundaries stream asynchronously
if (process.env.NODE_ENV !== "production") __renderEnd = performance.now();
} catch (ssrErr) {
Expand Down
Loading
Loading