Skip to content
Open
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
35 changes: 30 additions & 5 deletions packages/vinext/src/server/app-browser-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
commitClientNavigationState,
consumePrefetchResponse,
createClientNavigationRenderSnapshot,
getBfcacheIdMapContext,
getCurrentNextUrl,
getCurrentInterceptionContext,
getClientNavigationRenderContext,
Expand Down Expand Up @@ -62,6 +63,8 @@ import {
} from "./app-elements.js";
import {
createHistoryStateWithPreviousNextUrl,
createInitialBfcacheIdMap,
readHistoryStateBfcacheIds,
readHistoryStatePreviousNextUrl,
resolveInterceptionContextFromPreviousNextUrl,
resolveServerActionRequestState,
Expand Down Expand Up @@ -136,6 +139,7 @@ const VISITED_RESPONSE_CACHE_TTL = 5 * 60_000;
const MAX_TRAVERSAL_CACHE_TTL = 30 * 60_000;
const browserNavigationController = createAppBrowserNavigationController();
const NavigationCommitSignal = browserNavigationController.NavigationCommitSignal;
const BfcacheIdMapContext = getBfcacheIdMapContext();

// Parses a URI-encoded JSON value carried in a response header (e.g.
// `X-Vinext-Params`). Returns `null` on missing or malformed input so callers
Expand Down Expand Up @@ -217,13 +221,14 @@ function clearClientNavigationCaches(): void {
}

function createNavigationCommitEffect(options: {
bfcacheIds: Readonly<Record<string, string>>;
href: string;
historyUpdateMode: HistoryUpdateMode | undefined;
navId: number;
params: Record<string, string | string[]>;
previousNextUrl: string | null;
}): () => void {
const { href, historyUpdateMode, navId, params, previousNextUrl } = options;
const { bfcacheIds, href, historyUpdateMode, navId, params, previousNextUrl } = options;

return () => {
// Only update URL if this is still the active navigation.
Expand All @@ -241,12 +246,17 @@ function createNavigationCommitEffect(options: {
const historyState = createHistoryStateWithPreviousNextUrl(
preserveExistingState ? window.history.state : null,
previousNextUrl,
bfcacheIds,
);

if (historyUpdateMode === "replace" && window.location.href !== targetHref) {
replaceHistoryStateWithoutNotify(historyState, "", href);
} else if (historyUpdateMode === "push" && window.location.href !== targetHref) {
pushHistoryStateWithoutNotify(historyState, "", href);
} else if (historyUpdateMode === 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.

This new else if arm fires for traversals (historyUpdateMode === undefined) to persist the updated bfcacheIds into history.state even when the URL hasn't changed. Makes sense — but it also fires for refresh navigations (which also pass undefined for historyUpdateMode).

For refresh, this replaceState is harmless (same URL, same-ish state), but it's a behavioral change from before this PR where refresh commits didn't touch history state at all. Worth a comment clarifying that this is intentional for both traverse and refresh cases.

// Traversal and refresh commits don't change the URL, but still persist
// the latest bfcache id map so future history entries can restore it.
replaceHistoryStateWithoutNotify(historyState, "", window.location.href);
}

// URL has been updated; the recovery hard-nav target is no longer needed.
Expand All @@ -266,6 +276,7 @@ async function renderNavigationPayload(
pendingRouterState: PendingBrowserRouterState | null,
actionType: "navigate" | "replace" | "traverse" = "navigate",
operationLane: OperationLane = "navigation",
restoredBfcacheIds?: Readonly<Record<string, string>> | null,
): Promise<NavigationPayloadOutcome> {
try {
return await browserNavigationController.renderNavigationPayload({
Expand All @@ -281,6 +292,7 @@ async function renderNavigationPayload(
params,
pendingRouterState,
previousNextUrl,
restoredBfcacheIds,
targetHref,
navId,
});
Expand Down Expand Up @@ -461,6 +473,7 @@ function BrowserRoot({
const initialMetadata = AppElementsWire.readMetadata(resolvedElements);
const [treeStateValue, setTreeStateValue] = useState<AppRouterState | Promise<AppRouterState>>({
activeOperation: null,
bfcacheIds: createInitialBfcacheIdMap(resolvedElements),
elements: resolvedElements,
interceptionContext: initialMetadata.interceptionContext,
layoutIds: initialMetadata.layoutIds,
Expand Down Expand Up @@ -510,13 +523,17 @@ function BrowserRoot({
}

replaceHistoryStateWithoutNotify(
createHistoryStateWithPreviousNextUrl(window.history.state, treeState.previousNextUrl),
createHistoryStateWithPreviousNextUrl(
window.history.state,
treeState.previousNextUrl,
treeState.bfcacheIds,
),
"",
window.location.href,
);
}, [treeState.previousNextUrl, treeState.renderId]);
}, [treeState.bfcacheIds, treeState.previousNextUrl, treeState.renderId]);

const innerTree = createElement(
const routeTree = createElement(
RedirectBoundary,
null,
createElement(
Expand All @@ -530,6 +547,10 @@ function BrowserRoot({
),
);

const innerTree = BfcacheIdMapContext
? createElement(BfcacheIdMapContext.Provider, { value: treeState.bfcacheIds }, routeTree)
: routeTree;

// In dev, wrap the route tree in a top-level recovery boundary. A render
// error (e.g. a slot's RSC reference rejects) is caught here instead of
// tearing down BrowserRoot, so HMR can dispatch the next payload —
Expand Down Expand Up @@ -913,7 +934,7 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
latestClientParams,
);
replaceHistoryStateWithoutNotify(
createHistoryStateWithPreviousNextUrl(window.history.state, null),
createHistoryStateWithPreviousNextUrl(window.history.state, null, null),
"",
window.location.href,
);
Expand Down Expand Up @@ -999,6 +1020,8 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
const requestState = getRequestState(navigationKind, currentPrevNextUrl);
const requestInterceptionContext = requestState.interceptionContext;
const requestPreviousNextUrl = requestState.previousNextUrl;
const restoredBfcacheIds =
navigationKind === "traverse" ? readHistoryStateBfcacheIds(window.history.state) : null;

// Set this navigation as the pending pathname, overwriting any previous.
// Pass navId so only this navigation (or a newer one) can clear it later.
Expand Down Expand Up @@ -1056,6 +1079,7 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
pendingRouterState,
toActionType(navigationKind),
toOperationLane(navigationKind),
restoredBfcacheIds,
);
return;
}
Expand Down Expand Up @@ -1210,6 +1234,7 @@ function bootstrapHydration(rscStream: ReadableStream<Uint8Array>): void {
pendingRouterState,
toActionType(navigationKind),
toOperationLane(navigationKind),
restoredBfcacheIds,
);
if (renderOutcome !== "committed") return;
// Don't cache the response if this navigation was superseded during
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type NavigationPayloadOutcome = "committed" | "no-commit" | "hard-navigat
type HardNavigationMode = "assign" | "replace";

type BrowserNavigationCommitEffectFactory = (options: {
bfcacheIds: Readonly<Record<string, string>>;
href: string;
historyUpdateMode: HistoryUpdateMode | undefined;
navId: number;
Expand Down Expand Up @@ -68,6 +69,7 @@ type BrowserNavigationController = {
params: Record<string, string | string[]>;
pendingRouterState: PendingBrowserRouterState | null;
previousNextUrl: string | null;
restoredBfcacheIds?: Readonly<Record<string, string>> | null;
targetHref: string;
navId: number;
}): Promise<NavigationPayloadOutcome>;
Expand Down Expand Up @@ -462,6 +464,7 @@ export function createAppBrowserNavigationController(
params: Record<string, string | string[]>;
pendingRouterState: PendingBrowserRouterState | null;
previousNextUrl: string | null;
restoredBfcacheIds?: Readonly<Record<string, string>> | null;
targetHref: string;
navId: number;
}): Promise<NavigationPayloadOutcome> {
Expand All @@ -482,6 +485,7 @@ export function createAppBrowserNavigationController(
operationLane: options.operationLane,
previousNextUrl: options.previousNextUrl,
renderId,
restoredBfcacheIds: options.restoredBfcacheIds,
type: options.actionType,
});

Expand Down Expand Up @@ -515,6 +519,7 @@ export function createAppBrowserNavigationController(
renderId,
options.createNavigationCommitEffect({
href: options.targetHref,
bfcacheIds: approvedCommit.action.bfcacheIds,
historyUpdateMode: options.historyUpdateMode,
navId: options.navId,
params: options.params,
Expand Down
Loading
Loading