Skip to content

fix(app-router): model RSC redirect and traversal lifecycle #1248

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

Open
NathanDrake2406 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(app-router): model RSC redirect and traversal lifecycle #1248

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
6 changes: 6 additions & 0 deletions packages/vinext/src/global.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ declare global {
* @param redirectDepth - Internal parameter used to detect redirect loops.
* @param navigationKind - Internal hint for traversal vs regular navigation.
* @param historyUpdateMode - Internal hint for when history should publish.
* @param traversalIntent - Internal popstate direction/history metadata.
*/
__VINEXT_RSC_NAVIGATE__:
| ((
Expand All @@ -101,6 +102,11 @@ declare global {
historyUpdateMode?: "push" | "replace",
previousNextUrlOverride?: string | null,
programmaticTransition?: boolean,
traversalIntent?: {
direction: "back" | "forward" | "unknown";
historyState: unknown;
targetHistoryIndex: number | null;
},
) => Promise<void>)
| undefined;

Expand Down
144 changes: 107 additions & 37 deletions packages/vinext/src/server/app-browser-entry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,14 @@ import {
type AppWireElements,
} from "./app-elements.js";
import {
createHistoryStateWithPreviousNextUrl,
createHistoryStateWithNavigationMetadata,
readHistoryStatePreviousNextUrl,
readHistoryStateTraversalIndex,
resolveHistoryTraversalIntent,
resolveInterceptionContextFromPreviousNextUrl,
resolveServerActionRequestState,
type AppRouterState,
type HistoryTraversalIntent,
type OperationLane,
} from "./app-browser-state.js";
import { createPopstateRestoreHandler } from "./app-browser-popstate.js";
Expand All @@ -96,12 +99,11 @@ import {
getVinextRscCompatibilityId,
resolveHardNavigationTargetFromRscResponse,
resolveRscCompatibilityNavigationDecision,
stripRscCacheBustingSearchParam,
stripRscSuffix,
VINEXT_RSC_COMPATIBILITY_ID_HEADER,
VINEXT_RSC_CONTENT_TYPE,
} from "./app-rsc-cache-busting.js";
import { APP_RSC_RENDER_MODE_REFRESH_PRESERVE_UI } from "./app-rsc-render-mode.js";
import { resolveRscRedirectLifecycleHop } from "./app-browser-rsc-redirect.js";
import {
ACTION_REDIRECT_HEADER,
ACTION_REDIRECT_TYPE_HEADER,
Expand Down Expand Up @@ -190,6 +192,36 @@ let browserRouterStateHasEverCommitted = false;
// of stranding them on the previous URL with a blank page. Cleared once the
// commit effect runs (URL update succeeded) or the navigation is superseded.
let pendingNavigationRecoveryHref: string | null = null;
let currentHistoryTraversalIndex: number | null =
readHistoryStateTraversalIndex(window.history.state) ?? 0;
let nextHistoryTraversalIndex: number = currentHistoryTraversalIndex;

function allocateNavigationHistoryTraversalIndex(
historyUpdateMode: HistoryUpdateMode | undefined,
): number | null {
switch (historyUpdateMode) {
case "push":
return nextHistoryTraversalIndex + 1;
Comment thread
NathanDrake2406 marked this conversation as resolved.
case "replace":
return currentHistoryTraversalIndex;
case undefined:
return null;
default: {
const _exhaustive: never = historyUpdateMode;
throw new Error("[vinext] Unknown history update mode: " + String(_exhaustive));
}
}
}

function commitHistoryTraversalIndex(index: number | null): void {
currentHistoryTraversalIndex = index;
if (index !== null) {
// Keep allocation anchored to the highest app-owned entry we know about.
// Traversing to metadata-less entries makes the current index unknown, but
// the next app-owned push should still continue from known app history.
nextHistoryTraversalIndex = Math.max(nextHistoryTraversalIndex, index);
}
}

function getBrowserRouterState(): AppRouterState {
return browserNavigationController.getBrowserRouterState();
Expand Down Expand Up @@ -251,8 +283,9 @@ function createNavigationCommitEffect(options: {
navId: number;
params: Record<string, string | string[]>;
previousNextUrl: string | null;
targetHistoryIndex?: number | null;
}): () => void {
const { href, historyUpdateMode, navId, params, previousNextUrl } = options;
const { href, historyUpdateMode, navId, params, previousNextUrl, targetHistoryIndex } = options;

return () => {
// Only update URL if this is still the active navigation.
Expand All @@ -267,15 +300,26 @@ function createNavigationCommitEffect(options: {
const targetHref = new URL(href, window.location.origin).href;
stageClientParams(params);
const preserveExistingState = historyUpdateMode === "replace";
const historyState = createHistoryStateWithPreviousNextUrl(
const navigationHistoryIndex =
targetHistoryIndex !== undefined
? targetHistoryIndex
: allocateNavigationHistoryTraversalIndex(historyUpdateMode);
const historyState = createHistoryStateWithNavigationMetadata(
preserveExistingState ? window.history.state : null,
previousNextUrl,
{
previousNextUrl,
traversalIndex: navigationHistoryIndex,
},
);

if (historyUpdateMode === "replace" && window.location.href !== targetHref) {
replaceHistoryStateWithoutNotify(historyState, "", href);
commitHistoryTraversalIndex(navigationHistoryIndex);
} else if (historyUpdateMode === "push" && window.location.href !== targetHref) {
pushHistoryStateWithoutNotify(historyState, "", href);
commitHistoryTraversalIndex(navigationHistoryIndex);
} else if (targetHistoryIndex !== undefined) {
commitHistoryTraversalIndex(targetHistoryIndex);
}

// URL has been updated; the recovery hard-nav target is no longer needed.
Expand All @@ -295,6 +339,7 @@ async function renderNavigationPayload(
pendingRouterState: PendingBrowserRouterState | null,
actionType: "navigate" | "replace" | "traverse" = "navigate",
operationLane: OperationLane = "navigation",
traversalIntent: HistoryTraversalIntent | null = null,
): Promise<NavigationPayloadOutcome> {
try {
return await browserNavigationController.renderNavigationPayload({
Expand All @@ -310,6 +355,7 @@ async function renderNavigationPayload(
params,
pendingRouterState,
previousNextUrl,
targetHistoryIndex: traversalIntent === null ? undefined : traversalIntent.targetHistoryIndex,
targetHref,
navId,
});
Expand Down Expand Up @@ -424,6 +470,7 @@ type NavigationRequestState = {
function getRequestState(
navigationKind: NavigationKind,
previousNextUrlOverride?: string | null,
traverseHistoryState?: unknown,
): NavigationRequestState {
if (previousNextUrlOverride !== undefined) {
return {
Expand All @@ -442,7 +489,9 @@ function getRequestState(
previousNextUrl: getCurrentNextUrl(),
};
case "traverse": {
const previousNextUrl = readHistoryStatePreviousNextUrl(window.history.state);
const previousNextUrl = readHistoryStatePreviousNextUrl(
traverseHistoryState ?? window.history.state,
);
return {
interceptionContext: resolveInterceptionContextFromPreviousNextUrl(
previousNextUrl,
Expand Down Expand Up @@ -555,7 +604,10 @@ function BrowserRoot({
}

replaceHistoryStateWithoutNotify(
createHistoryStateWithPreviousNextUrl(window.history.state, treeState.previousNextUrl),
createHistoryStateWithNavigationMetadata(window.history.state, {
previousNextUrl: treeState.previousNextUrl,
traversalIndex: currentHistoryTraversalIndex,
}),
"",
window.location.href,
);
Expand Down Expand Up @@ -961,7 +1013,10 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
latestClientParams,
);
replaceHistoryStateWithoutNotify(
createHistoryStateWithPreviousNextUrl(window.history.state, null),
createHistoryStateWithNavigationMetadata(window.history.state, {
previousNextUrl: null,
traversalIndex: currentHistoryTraversalIndex,
}),
"",
window.location.href,
);
Expand Down Expand Up @@ -1009,6 +1064,7 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
historyUpdateMode?: HistoryUpdateMode,
previousNextUrlOverride?: string | null,
programmaticTransition = false,
traversalIntent?: HistoryTraversalIntent,
): Promise<void> {
let pendingRouterState: PendingBrowserRouterState | null = null;
// Hoist navId above try so the catch and finally blocks can reference it.
Expand All @@ -1022,6 +1078,14 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
let currentHistoryMode = historyUpdateMode;
let currentPrevNextUrl = previousNextUrlOverride;
let redirectCount = redirectDepth;
const activeTraversalIntent =
navigationKind === "traverse"
? (traversalIntent ??
resolveHistoryTraversalIntent({
currentHistoryIndex: currentHistoryTraversalIndex,
historyState: window.history.state,
}))
: null;

try {
const shouldUsePendingRouterState = programmaticTransition;
Expand All @@ -1037,16 +1101,12 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
}

while (true) {
if (redirectCount > 10) {
console.error(
"[vinext] Too many RSC redirects — aborting navigation to prevent infinite loop.",
);
window.location.href = currentHref;
return;
}

const url = new URL(currentHref, window.location.origin);
const requestState = getRequestState(navigationKind, currentPrevNextUrl);
const requestState = getRequestState(
navigationKind,
currentPrevNextUrl,
activeTraversalIntent?.historyState,
);
const requestInterceptionContext = requestState.interceptionContext;
const requestPreviousNextUrl = requestState.previousNextUrl;

Expand Down Expand Up @@ -1117,6 +1177,7 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
pendingRouterState,
toActionType(navigationKind),
toOperationLane(navigationKind),
activeTraversalIntent,
);
return;
}
Expand Down Expand Up @@ -1190,26 +1251,34 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
return;
}

const finalUrl = new URL(navResponseUrl ?? navResponse.url, window.location.origin);
stripRscCacheBustingSearchParam(finalUrl);
const requestedUrl = new URL(rscUrl, window.location.origin);

if (finalUrl.pathname !== requestedUrl.pathname) {
// Server-side redirect: update the URL in history and loop to fetch
// the destination without settling pendingRouterState. This keeps
// isPending true across all redirect hops instead of flashing false.
const destinationPath = stripRscSuffix(finalUrl.pathname) + finalUrl.search;
replaceHistoryStateWithoutNotify(
createHistoryStateWithPreviousNextUrl(null, requestPreviousNextUrl),
"",
destinationPath,
);
const redirectDecision = resolveRscRedirectLifecycleHop({
currentHref,
historyUpdateMode: currentHistoryMode ?? "replace",
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot May 16, 2026

Choose a reason for hiding this comment

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

The fallback currentHistoryMode ?? "replace" means that if the redirect decision helper receives historyUpdateMode: undefined (which happens after a previous redirect hop set currentHistoryMode = undefined), it gets coerced to "replace". But the follow decision then propagates this "replace" back as redirectDecision.historyUpdateMode, overwriting the loop variable.

On a multi-hop chain: first hop enters with the original mode (e.g. "push"), follow preserves "push". On the second hop, if the first hop's follow already set currentHistoryMode = "push", we're fine. But re-check: the first hop's follow sets currentHistoryMode = redirectDecision.historyUpdateMode which is the input options.historyUpdateMode ("push"). Second hop: currentHistoryMode is still "push", so "push" ?? "replace" = "push". Correct.

The ?? "replace" only fires if historyUpdateMode was previously set to undefined — which only happens in the old code path (now removed). With the new code, historyUpdateMode stays as the original value through all hops. The fallback is defensive. This is fine.

origin: window.location.origin,
redirectDepth: redirectCount,
requestPreviousNextUrl,
responseUrl: navResponseUrl ?? navResponse.url,
});

if (redirectDecision.kind === "terminal-hard-navigation") {
if (redirectDecision.reason === "maxRedirectsExceeded") {
console.error(
"[vinext] Too many RSC redirects — aborting navigation to prevent infinite loop.",
);
}
window.location.href = redirectDecision.href;
return;
}

currentHref = destinationPath;
// URL already written above; the commit effect must not push/replace again.
currentHistoryMode = undefined;
currentPrevNextUrl = requestPreviousNextUrl;
redirectCount += 1;
if (redirectDecision.kind === "follow") {
// Server-side redirect: keep the redirect chain inside this operation
// and defer URL/history mutation to the eventual approved commit.
// This keeps isPending true across all hops and avoids publishing a
// destination URL before its RSC payload is lifecycle-approved.
currentHref = redirectDecision.href;
currentHistoryMode = redirectDecision.historyUpdateMode;
currentPrevNextUrl = redirectDecision.previousNextUrl;
redirectCount = redirectDecision.redirectDepth;
continue;
}

Expand Down Expand Up @@ -1264,6 +1333,7 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
pendingRouterState,
toActionType(navigationKind),
toOperationLane(navigationKind),
activeTraversalIntent,
);
if (renderOutcome !== "committed") return;
// Don't cache the response if this navigation was superseded during
Expand Down
4 changes: 4 additions & 0 deletions packages/vinext/src/server/app-browser-navigation-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type BrowserNavigationCommitEffectFactory = (options: {
navId: number;
params: Record<string, string | string[]>;
previousNextUrl: string | null;
targetHistoryIndex?: number | null;
}) => () => void;

type BrowserRouterStateRef = {
Expand Down Expand Up @@ -80,6 +81,7 @@ type BrowserNavigationController = {
params: Record<string, string | string[]>;
pendingRouterState: PendingBrowserRouterState | null;
previousNextUrl: string | null;
targetHistoryIndex?: number | null;
targetHref: string;
navId: number;
}): Promise<NavigationPayloadOutcome>;
Expand Down Expand Up @@ -488,6 +490,7 @@ export function createAppBrowserNavigationController(
params: Record<string, string | string[]>;
pendingRouterState: PendingBrowserRouterState | null;
previousNextUrl: string | null;
targetHistoryIndex?: number | null;
targetHref: string;
navId: number;
}): Promise<NavigationPayloadOutcome> {
Expand Down Expand Up @@ -545,6 +548,7 @@ export function createAppBrowserNavigationController(
navId: options.navId,
params: options.params,
previousNextUrl: approvedCommit.previousNextUrl,
targetHistoryIndex: options.targetHistoryIndex,
}),
);
activateNavigationSnapshot();
Expand Down
Loading
Loading