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
43 changes: 40 additions & 3 deletions packages/vinext/src/entries/app-rsc-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,21 @@ function __isrCacheKey(pathname, suffix) {
return prefix + ":__hash:" + __isrFnv1a64(normalized) + ":" + suffix;
}
function __isrHtmlKey(pathname) { return __isrCacheKey(pathname, "html"); }
function __isrRscKey(pathname) { return __isrCacheKey(pathname, "rsc"); }
function __isrRscKey(pathname, mountedSlotsHeader) {
if (!mountedSlotsHeader) return __isrCacheKey(pathname, "rsc");
return __isrCacheKey(pathname, "rsc:" + __isrFnv1a64(mountedSlotsHeader));
}
function __normalizeMountedSlotsHeader(raw) {
if (!raw) return null;
const normalized = Array.from(
new Set(
raw
.split(/\\s+/)
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.

Previous reviews flagged the regex divergence between this generated code and the typed module. This .split(/\\s+/) in the template string produces .split(/\s+/) in the generated output, which correctly matches app-elements.ts:28. Just confirming this is already aligned — the earlier review suggestion to change from " " to /\\s+/ appears to have been addressed (or was never an issue in the committed code).

.filter(Boolean),
),
).sort().join(" ");
return normalized || null;
}
function __isrRouteKey(pathname) { return __isrCacheKey(pathname, "route"); }
// Verbose cache logging — opt in with NEXT_PRIVATE_DEBUG_CACHE=1.
// Matches the env var Next.js uses for its own cache debug output so operators
Expand Down Expand Up @@ -911,7 +925,7 @@ function findIntercept(pathname) {
return null;
}

async function buildPageElements(route, params, routePath, opts, searchParams) {
async function buildPageElements(route, params, routePath, opts, searchParams, isRscRequest, request) {
const PageComponent = route.page?.default;
if (!PageComponent) {
const _noExportRouteId = "route:" + routePath;
Expand Down Expand Up @@ -1024,9 +1038,18 @@ async function buildPageElements(route, params, routePath, opts, searchParams) {
// dynamic, and this avoids false positives from React internals.
if (hasSearchParams) markDynamicUsage();
}
const __mountedSlotsHeader = __normalizeMountedSlotsHeader(
request?.headers?.get("x-vinext-mounted-slots"),
);
const mountedSlotIds = __mountedSlotsHeader
? new Set(__mountedSlotsHeader.split(" "))
: null;

return __buildAppPageElements({
element: createElement(PageComponent, pageProps),
globalErrorModule: ${globalErrorVar ? globalErrorVar : "null"},
isRscRequest,
mountedSlotIds,
makeThenableParams,
matchedParams: params,
resolvedMetadata,
Expand Down Expand Up @@ -1778,6 +1801,8 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
cleanPathname,
undefined,
url.searchParams,
isRscRequest,
request,
);
} else {
const _actionRouteId = "route:" + cleanPathname;
Expand Down Expand Up @@ -2099,6 +2124,9 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {

// force-dynamic: set no-store Cache-Control
const isForceDynamic = dynamicConfig === "force-dynamic";
const __mountedSlotsHeader = __normalizeMountedSlotsHeader(
request.headers.get("x-vinext-mounted-slots"),
);

// ── ISR cache read (production only) ─────────────────────────────────────
// Read from cache BEFORE generateStaticParams and all rendering work.
Expand Down Expand Up @@ -2132,6 +2160,7 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
isrHtmlKey: __isrHtmlKey,
isrRscKey: __isrRscKey,
isrSet: __isrSet,
mountedSlotsHeader: __mountedSlotsHeader,
revalidateSeconds,
renderFreshPageForCache: async function() {
// Re-render the page to produce fresh HTML + RSC data for the cache
Expand All @@ -2146,12 +2175,17 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
return _runWithUnifiedCtx(__revalUCtx, async () => {
_ensureFetchPatch();
setNavigationContext({ pathname: cleanPathname, searchParams: new URLSearchParams(), params });
// Slot context (X-Vinext-Mounted-Slots) is inherited from the
// triggering request so the regen result is cached under the
// correct slot-variant key.
const __revalElement = await buildPageElements(
route,
params,
cleanPathname,
undefined,
new URLSearchParams(),
isRscRequest,
request,
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 original request is passed here, so the background regen renders with the triggering client's X-Vinext-Mounted-Slots header. The result is cached under the slot-variant key, which is correct for that specific variant — but the existing comment block (lines 2167-2170) describes avoiding user-specific context only in terms of headers/cookies, not slot state. Worth a one-liner noting that slot context is intentionally inherited:

Suggested change
request,
isRscRequest,
request, // Slot context (X-Vinext-Mounted-Slots) is intentionally
// inherited from the triggering request — the regen result
// is cached under the slot-variant key.

);
const __revalOnError = createRscOnErrorHandler(request, cleanPathname, route.pattern);
const __revalRscStream = renderToReadableStream(__revalElement, { onError: __revalOnError });
Expand Down Expand Up @@ -2208,6 +2242,8 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
cleanPathname,
interceptOpts,
interceptSearchParams,
isRscRequest,
request,
);
},
cleanPathname,
Expand Down Expand Up @@ -2260,7 +2296,7 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {

const __pageBuildResult = await __buildAppPageElement({
buildPageElement() {
return buildPageElements(route, params, cleanPathname, interceptOpts, url.searchParams);
return buildPageElements(route, params, cleanPathname, interceptOpts, url.searchParams, isRscRequest, request);
},
renderErrorBoundaryPage(buildErr) {
return renderErrorBoundaryPage(route, buildErr, isRscRequest, request, params, _scriptNonce);
Expand Down Expand Up @@ -2357,6 +2393,7 @@ async function _handleRequest(request, __reqCtx, _mwCtx) {
return PageComponent({ params: _asyncLayoutParams, searchParams: _asyncSearchParams });
},
revalidateSeconds,
mountedSlotsHeader: __mountedSlotsHeader,
renderErrorBoundaryResponse(renderErr) {
return renderErrorBoundaryPage(route, renderErr, isRscRequest, request, params, _scriptNonce);
},
Expand Down
28 changes: 25 additions & 3 deletions packages/vinext/src/server/app-browser-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
restoreRscResponse,
setClientParams,
snapshotRscResponse,
setMountedSlotsHeader,
setNavigationContext,
toRscUrl,
type CachedRscResponse,
Expand All @@ -47,6 +48,7 @@ import {
getVinextBrowserGlobal,
} from "./app-browser-stream.js";
import {
getMountedSlotIdsHeader,
normalizeAppElements,
readAppElementsMetadata,
type AppElements,
Expand Down Expand Up @@ -218,13 +220,19 @@ function evictVisitedResponseCacheIfNeeded(): void {

function getVisitedResponse(
rscUrl: string,
mountedSlotsHeader: string | null,
navigationKind: NavigationKind,
): VisitedResponseCacheEntry | null {
const cached = visitedResponseCache.get(rscUrl);
if (!cached) {
return null;
}
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 slot-mismatch path unconditionally deletes the visited-response entry. Since the visited cache is keyed by URL (not URL+slots), this means rapid navigation between two slot contexts for the same URL will always miss. This is the simplest correct behavior given the single-key structure, but if visited-response cache misses become a perf concern for parallel-route-heavy apps, the cache could be changed to store multiple slot variants per URL.

Not blocking — just noting the tradeoff.


if ((cached.response.mountedSlotsHeader ?? null) !== mountedSlotsHeader) {
visitedResponseCache.delete(rscUrl);
return null;
}

if (navigationKind === "refresh") {
return null;
}
Expand Down Expand Up @@ -411,9 +419,14 @@ function BrowserRoot({
if (browserRouterStateRef === stateRef) {
browserRouterStateRef = null;
}
setMountedSlotsHeader(null);
};
}, [dispatchTreeState]);

useLayoutEffect(() => {
setMountedSlotsHeader(getMountedSlotIdsHeader(stateRef.current.elements));
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: this reactive effect correctly tracks element changes independently from the stable dispatch/ref setup effect above. Splitting into two effects avoids unnecessary teardown/re-setup of dispatchBrowserRouterAction and browserRouterStateRef on every navigation.

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: splitting into two effects is the right call. The first effect (deps: [dispatchTreeState]) handles stable module-level ref setup/teardown. This reactive effect only runs when treeState.elements changes, avoiding unnecessary teardown/re-setup of dispatchBrowserRouterAction and browserRouterStateRef on every navigation.

}, [treeState.elements]);

const committedTree = createElement(
NavigationCommitSignal,
{ renderId: treeState.renderId },
Expand Down Expand Up @@ -737,7 +750,9 @@ async function main(): Promise<void> {
const isSameRoute =
stripBasePath(url.pathname, __basePath) ===
stripBasePath(window.location.pathname, __basePath);
const cachedRoute = getVisitedResponse(rscUrl, navigationKind);
const elementsAtNavStart = getBrowserRouterState().elements;
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: capturing elementsAtNavStart before any async work ensures the mounted-slot header reflects the state at navigation start, not a potentially-mutated state from a concurrent navigation that committed during the fetch. This is the right approach for consistency between the header sent to the server and the compatibility check against prefetched responses.

const mountedSlotsHeader = getMountedSlotIdsHeader(elementsAtNavStart);
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.

Capturing elementsAtNavStart before any async work is the right approach. This ensures the mounted-slot header reflects the state at navigation start, not a potentially-mutated state from a concurrent navigation that committed during the fetch. The same captured value is used for both the fetch header and the prefetch compatibility check — consistent.

const cachedRoute = getVisitedResponse(rscUrl, mountedSlotsHeader, navigationKind);
if (cachedRoute) {
// Check stale-navigation before and after createFromFetch. The pre-check
// avoids wasted parse work; the post-check catches supersessions that
Expand Down Expand Up @@ -777,19 +792,26 @@ async function main(): Promise<void> {
return;
}

// Continue using the slot state captured at navigation start for fetches
// and prefetch compatibility decisions.

let navResponse: Response | undefined;
let navResponseUrl: string | null = null;
if (navigationKind !== "refresh") {
const prefetchedResponse = consumePrefetchResponse(rscUrl);
const prefetchedResponse = consumePrefetchResponse(rscUrl, mountedSlotsHeader);
if (prefetchedResponse) {
navResponse = restoreRscResponse(prefetchedResponse, false);
navResponseUrl = prefetchedResponse.url;
}
}

if (!navResponse) {
const rscFetchHeaders: Record<string, string> = { Accept: "text/x-component" };
if (mountedSlotsHeader) {
rscFetchHeaders["X-Vinext-Mounted-Slots"] = mountedSlotsHeader;
}
navResponse = await fetch(rscUrl, {
headers: { Accept: "text/x-component" },
headers: rscFetchHeaders,
credentials: "include",
});
}
Expand Down
25 changes: 25 additions & 0 deletions packages/vinext/src/server/app-elements.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,31 @@ export type AppElementsMetadata = {
rootLayoutTreePath: string | null;
};

export function normalizeMountedSlotsHeader(header: string | null | undefined): string | null {
if (!header) {
return null;
}

const slotIds = Array.from(new Set(header.split(/\s+/).filter(Boolean))).sort();

return slotIds.length > 0 ? slotIds.join(" ") : null;
}

export function getMountedSlotIds(elements: AppElements): string[] {
return Object.keys(elements)
.filter((key) => {
const value = elements[key];
return (
key.startsWith("slot:") && value !== null && value !== undefined && value !== UNMATCHED_SLOT
);
})
.sort();
}

export function getMountedSlotIdsHeader(elements: AppElements): string | null {
return normalizeMountedSlotsHeader(getMountedSlotIds(elements).join(" "));
}

export function normalizeAppElements(elements: AppWireElements): AppElements {
let needsNormalization = false;
for (const [key, value] of Object.entries(elements)) {
Expand Down
63 changes: 43 additions & 20 deletions packages/vinext/src/server/app-page-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type AppPageCacheRenderResult = {
export type BuildAppPageCachedResponseOptions = {
cacheState: "HIT" | "STALE";
isRscRequest: boolean;
mountedSlotsHeader?: string | null;
revalidateSeconds: number;
};

Expand All @@ -30,8 +31,9 @@ export type ReadAppPageCacheResponseOptions = {
isrDebug?: AppPageDebugLogger;
isrGet: AppPageCacheGetter;
isrHtmlKey: (pathname: string) => string;
isrRscKey: (pathname: string) => string;
isrRscKey: (pathname: string, mountedSlotsHeader?: string | null) => string;
isrSet: AppPageCacheSetter;
mountedSlotsHeader?: string | null;
revalidateSeconds: number;
renderFreshPageForCache: () => Promise<AppPageCacheRenderResult>;
scheduleBackgroundRegeneration: AppPageBackgroundRegenerator;
Expand All @@ -43,7 +45,7 @@ export type FinalizeAppPageHtmlCacheResponseOptions = {
getPageTags: () => string[];
isrDebug?: AppPageDebugLogger;
isrHtmlKey: (pathname: string) => string;
isrRscKey: (pathname: string) => string;
isrRscKey: (pathname: string, mountedSlotsHeader?: string | null) => string;
isrSet: AppPageCacheSetter;
revalidateSeconds: number;
waitUntil?: (promise: Promise<void>) => void;
Expand All @@ -56,8 +58,9 @@ export type ScheduleAppPageRscCacheWriteOptions = {
dynamicUsedDuringBuild: boolean;
getPageTags: () => string[];
isrDebug?: AppPageDebugLogger;
isrRscKey: (pathname: string) => string;
isrRscKey: (pathname: string, mountedSlotsHeader?: string | null) => string;
isrSet: AppPageCacheSetter;
mountedSlotsHeader?: string | null;
revalidateSeconds: number;
waitUntil?: (promise: Promise<void>) => void;
};
Expand Down Expand Up @@ -95,12 +98,17 @@ export function buildAppPageCachedResponse(
return null;
}

const rscHeaders: Record<string, string> = {
"Content-Type": "text/x-component; charset=utf-8",
...headers,
};
if (options.mountedSlotsHeader) {
rscHeaders["X-Vinext-Mounted-Slots"] = options.mountedSlotsHeader;
}

return new Response(cachedValue.rscData, {
status,
headers: {
"Content-Type": "text/x-component; charset=utf-8",
...headers,
},
headers: rscHeaders,
});
}

Expand All @@ -121,7 +129,7 @@ export async function readAppPageCacheResponse(
options: ReadAppPageCacheResponseOptions,
): Promise<Response | null> {
const isrKey = options.isRscRequest
? options.isrRscKey(options.cleanPathname)
? options.isrRscKey(options.cleanPathname, options.mountedSlotsHeader)
: options.isrHtmlKey(options.cleanPathname);

try {
Expand All @@ -132,6 +140,7 @@ export async function readAppPageCacheResponse(
const hitResponse = buildAppPageCachedResponse(cachedValue, {
cacheState: "HIT",
isRscRequest: options.isRscRequest,
mountedSlotsHeader: options.mountedSlotsHeader,
revalidateSeconds: options.revalidateSeconds,
});

Expand All @@ -151,29 +160,43 @@ export async function readAppPageCacheResponse(
// Preserve the legacy behavior from the inline generator: stale entries
// still trigger background regeneration even if this request cannot use
// the stale payload and will fall through to a fresh render.
// Dedup key is pathname-only: if multiple slot variants are stale
// concurrently, only one regen runs. Other variants refresh on
// their next STALE read.
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 pathname-only dedup key means if two concurrent RSC requests for the same URL with different slot combinations both get STALE, only one regen runs. The second variant stays stale until its next read. The comment here correctly documents this — confirming the tradeoff (fewer concurrent regens vs. delayed freshness for non-primary variants) is reasonable.

options.scheduleBackgroundRegeneration(options.cleanPathname, async () => {
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 dedup key here is options.cleanPathname alone, but the cache write at line 167 uses options.isrRscKey(options.cleanPathname, options.mountedSlotsHeader) which includes slot state. This means two concurrent RSC requests for the same URL with different slot combinations will have only one regen run (the loser's variant stays stale until its next STALE read).

This is probably the right tradeoff (avoids concurrent regen storms for slot-heavy pages), but a brief comment would help:

Suggested change
options.scheduleBackgroundRegeneration(options.cleanPathname, async () => {
options.scheduleBackgroundRegeneration(options.cleanPathname, async () => {
// Dedup key is pathname-only: if multiple slot variants are stale
// concurrently, only one regen runs. Other variants refresh on
// their next STALE read.

const revalidatedPage = await options.renderFreshPageForCache();

await Promise.all([
const writes = [
options.isrSet(
options.isrHtmlKey(options.cleanPathname),
buildAppPageCacheValue(revalidatedPage.html, undefined, 200),
options.revalidateSeconds,
revalidatedPage.tags,
),
options.isrSet(
options.isrRscKey(options.cleanPathname),
options.isrRscKey(options.cleanPathname, options.mountedSlotsHeader),
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 note: the renderFreshPageForCache closure (in the generated entry) uses isRscRequest and request from the outer scope, so the regen produces a slot-variant-specific RSC payload. The cache key at line 167 correctly uses options.mountedSlotsHeader to store it under the right variant. This means different clients with different slot combinations get independent cache entries — cache fan-out is proportional to the number of distinct slot combinations observed, which could be worth monitoring in production.

buildAppPageCacheValue("", revalidatedPage.rscData, 200),
options.revalidateSeconds,
revalidatedPage.tags,
),
]);
];

if (!options.isRscRequest) {
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.

When an RSC request triggers stale revalidation, only the RSC cache key (with the requesting client's mounted-slot variant) is regenerated — the HTML cache key is skipped.

This is correct for the immediate request, but it means an RSC-triggered regen won't refresh the HTML cache. If the HTML cache is also stale for the same pathname, it will require its own separate request to trigger regeneration. Might be worth a brief comment noting this is an intentional asymmetry.

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 asymmetry between RSC-triggered and HTML-triggered regens is correct: HTML cache is canonical (slot-independent), so an RSC-triggered regen should only refresh the requesting client's RSC slot variant. The comment explains this well. One thing worth noting: the renderFreshPageForCache closure in the generated entry inherits the triggering request's slot context (via the original request object), so the regen does produce a slot-variant-specific RSC payload — correctly cached under the matching variant key.

// HTML cache is slot-state-independent (canonical), so only refresh it
// during HTML-triggered regens. RSC-triggered regens only update the
// requesting client's RSC slot variant; a stale HTML cache entry will
// be regenerated independently by the next full-page HTML request.
writes.push(
Comment on lines +177 to +182
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 asymmetry here is correct but non-obvious. When an RSC request triggers stale regen, only the RSC slot-variant cache key is refreshed. The HTML cache (if also stale) requires a separate HTML request to trigger its own regen. Consider a brief comment:

Suggested change
if (!options.isRscRequest) {
// HTML remains canonical across slot-state variants; only RSC cache
// entries fan out by mounted-slot header.
writes.push(
if (!options.isRscRequest) {
// HTML cache is slot-state-independent (canonical), so only refresh it
// during HTML-triggered regens. RSC-triggered regens only update the
// requesting client's RSC slot variant; a stale HTML cache entry will
// be regenerated independently by the next full-page HTML request.

options.isrSet(
options.isrHtmlKey(options.cleanPathname),
buildAppPageCacheValue(revalidatedPage.html, undefined, 200),
options.revalidateSeconds,
revalidatedPage.tags,
),
);
}

await Promise.all(writes);
options.isrDebug?.("regen complete", options.cleanPathname);
});

const staleResponse = buildAppPageCachedResponse(cachedValue, {
cacheState: "STALE",
isRscRequest: options.isRscRequest,
mountedSlotsHeader: options.mountedSlotsHeader,
revalidateSeconds: options.revalidateSeconds,
});

Expand Down Expand Up @@ -209,7 +232,7 @@ export function finalizeAppPageHtmlCacheResponse(

const [streamForClient, streamForCache] = response.body.tee();
const htmlKey = options.isrHtmlKey(options.cleanPathname);
const rscKey = options.isrRscKey(options.cleanPathname);
const rscKey = options.isrRscKey(options.cleanPathname, null);
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.

finalizeAppPageHtmlCacheResponse correctly writes the RSC cache entry with mountedSlotsHeader: null — the HTML path produces a canonical (slot-independent) RSC payload. Consistent with the overall design.


const cachePromise = (async () => {
try {
Expand Down Expand Up @@ -272,7 +295,7 @@ export function scheduleAppPageRscCacheWrite(
return false;
}

const rscKey = options.isrRscKey(options.cleanPathname);
const rscKey = options.isrRscKey(options.cleanPathname, options.mountedSlotsHeader);
const cachePromise = (async () => {
try {
const rscData = await capturedRscDataPromise;
Expand Down
Loading
Loading