diff --git a/packages/vinext/src/server/app-browser-entry.ts b/packages/vinext/src/server/app-browser-entry.ts index bbcf9eaa9..15b5d0aa7 100644 --- a/packages/vinext/src/server/app-browser-entry.ts +++ b/packages/vinext/src/server/app-browser-entry.ts @@ -291,6 +291,7 @@ async function renderNavigationPayload( async function commitSameUrlNavigatePayload( nextElements: Promise, returnValue?: ServerActionResult["returnValue"], + actionInitiationState?: AppRouterState, ): Promise { const navigationSnapshot = createClientNavigationRenderSnapshot( window.location.href, @@ -300,6 +301,7 @@ async function commitSameUrlNavigatePayload( nextElements, navigationSnapshot, returnValue, + actionInitiationState, ); } @@ -853,10 +855,15 @@ function registerServerActionCallback(): void { return commitSameUrlNavigatePayload( Promise.resolve(AppElementsWire.decode(result.root)), result.returnValue, + currentState, ); } - return commitSameUrlNavigatePayload(Promise.resolve(AppElementsWire.decode(result))); + return commitSameUrlNavigatePayload( + Promise.resolve(AppElementsWire.decode(result)), + undefined, + currentState, + ); }); } diff --git a/packages/vinext/src/server/app-browser-navigation-controller.ts b/packages/vinext/src/server/app-browser-navigation-controller.ts index a46a83df8..b6b93cdfb 100644 --- a/packages/vinext/src/server/app-browser-navigation-controller.ts +++ b/packages/vinext/src/server/app-browser-navigation-controller.ts @@ -73,6 +73,7 @@ type BrowserNavigationController = { nextElements: Promise, navigationSnapshot: ClientNavigationRenderSnapshot, returnValue?: { ok: boolean; data: unknown }, + actionInitiationState?: AppRouterState, ): Promise; hmrReplaceTree( nextElements: Promise, @@ -417,7 +418,7 @@ export function createAppBrowserNavigationController( const approval = approvePendingNavigationCommit({ activeNavigationId, - currentState, + currentState: getBrowserRouterState(), pending, startedNavigationId: options.navId, }); @@ -476,14 +477,10 @@ export function createAppBrowserNavigationController( nextElements: Promise, navigationSnapshot: ClientNavigationRenderSnapshot, returnValue?: { ok: boolean; data: unknown }, + actionInitiationState?: AppRouterState, ): Promise { - const currentState = getBrowserRouterState(); + const currentState = actionInitiationState ?? getBrowserRouterState(); const startedNavigationId = activeNavigationId; - // Known limitation: if a same-URL navigation fully commits while this - // server action is awaiting resolveAndClassifyNavigationCommit(), the action - // can still dispatch its older payload afterward. The old pre-2c code had - // the same race, and Next.js has similar behavior. Tightening this would - // need a stronger commit-version gate than activeNavigationId alone. const { approvedCommit, decision, @@ -496,6 +493,7 @@ export function createAppBrowserNavigationController( currentState, navigationSnapshot, nextElements, + getCurrentStateForApproval: getBrowserRouterState, renderId: allocateRenderId(), operationLane: "server-action", startedNavigationId, diff --git a/packages/vinext/src/server/app-browser-state.ts b/packages/vinext/src/server/app-browser-state.ts index 69faf7776..75a9a23f7 100644 --- a/packages/vinext/src/server/app-browser-state.ts +++ b/packages/vinext/src/server/app-browser-state.ts @@ -277,14 +277,20 @@ export function shouldHardNavigate( export function resolvePendingNavigationCommitDisposition(options: { activeNavigationId: number; + currentVisibleCommitVersion: number; currentRootLayoutTreePath: string | null; nextRootLayoutTreePath: string | null; startedNavigationId: number; + startedVisibleCommitVersion: number; }): PendingNavigationCommitDisposition { if (options.startedNavigationId !== options.activeNavigationId) { return "skip"; } + if (options.startedVisibleCommitVersion !== options.currentVisibleCommitVersion) { + return "skip"; + } + if (shouldHardNavigate(options.currentRootLayoutTreePath, options.nextRootLayoutTreePath)) { return "hard-navigate"; } @@ -294,16 +300,20 @@ export function resolvePendingNavigationCommitDisposition(options: { export function resolvePendingNavigationCommitDispositionDecision(options: { activeNavigationId: number; + currentVisibleCommitVersion: number; currentRootLayoutTreePath: string | null; nextRootLayoutTreePath: string | null; startedNavigationId: number; + startedVisibleCommitVersion: number; }): PendingNavigationCommitDispositionDecision { const disposition = resolvePendingNavigationCommitDisposition(options); const traceFields = { activeNavigationId: options.activeNavigationId, + currentVisibleCommitVersion: options.currentVisibleCommitVersion, currentRootLayoutTreePath: options.currentRootLayoutTreePath, nextRootLayoutTreePath: options.nextRootLayoutTreePath, startedNavigationId: options.startedNavigationId, + startedVisibleCommitVersion: options.startedVisibleCommitVersion, }; return { diff --git a/packages/vinext/src/server/app-browser-visible-commit.ts b/packages/vinext/src/server/app-browser-visible-commit.ts index 2668ddf15..02f4ad61d 100644 --- a/packages/vinext/src/server/app-browser-visible-commit.ts +++ b/packages/vinext/src/server/app-browser-visible-commit.ts @@ -63,9 +63,11 @@ export function applyApprovedVisibleCommit( function resolvePendingNavigationCommitDecision(options: { activeNavigationId: number; + currentVisibleCommitVersion: number; currentRootLayoutTreePath: string | null; nextRootLayoutTreePath: string | null; startedNavigationId: number; + startedVisibleCommitVersion: number; }): CommitDecision { const { disposition, trace } = resolvePendingNavigationCommitDispositionDecision(options); @@ -123,9 +125,11 @@ export function approvePendingNavigationCommit(options: { }): CommitApproval { const decision = resolvePendingNavigationCommitDecision({ activeNavigationId: options.activeNavigationId, + currentVisibleCommitVersion: options.currentState.visibleCommitVersion, currentRootLayoutTreePath: options.currentState.rootLayoutTreePath, nextRootLayoutTreePath: options.pending.rootLayoutTreePath, startedNavigationId: options.startedNavigationId, + startedVisibleCommitVersion: options.pending.action.operation.startedVisibleCommitVersion, }); switch (decision.disposition) { @@ -155,6 +159,7 @@ export async function resolveAndClassifyNavigationCommit(options: { currentState: AppRouterState; navigationSnapshot: ClientNavigationRenderSnapshot; nextElements: Promise; + getCurrentStateForApproval?: () => AppRouterState; operationLane: OperationLane; previousNextUrl?: string | null; renderId: number; @@ -171,9 +176,10 @@ export async function resolveAndClassifyNavigationCommit(options: { type: options.type, }); + const approvalState = options.getCurrentStateForApproval?.() ?? options.currentState; const approval = approvePendingNavigationCommit({ activeNavigationId: options.activeNavigationId, - currentState: options.currentState, + currentState: approvalState, pending, startedNavigationId: options.startedNavigationId, }); diff --git a/tests/app-browser-entry.test.ts b/tests/app-browser-entry.test.ts index 0db3ceb88..7525dbad2 100644 --- a/tests/app-browser-entry.test.ts +++ b/tests/app-browser-entry.test.ts @@ -300,9 +300,11 @@ describe("app browser entry state helpers", () => { expect( resolvePendingNavigationCommitDisposition({ activeNavigationId: 3, + currentVisibleCommitVersion: currentState.visibleCommitVersion, currentRootLayoutTreePath: currentState.rootLayoutTreePath, nextRootLayoutTreePath: pending.rootLayoutTreePath, startedNavigationId: 3, + startedVisibleCommitVersion: pending.action.operation.startedVisibleCommitVersion, }), ).toBe("hard-navigate"); }); @@ -391,9 +393,11 @@ describe("app browser entry state helpers", () => { expect( resolvePendingNavigationCommitDisposition({ activeNavigationId: 5, + currentVisibleCommitVersion: currentState.visibleCommitVersion, currentRootLayoutTreePath: currentState.rootLayoutTreePath, nextRootLayoutTreePath: pending.rootLayoutTreePath, startedNavigationId: 4, + startedVisibleCommitVersion: pending.action.operation.startedVisibleCommitVersion, }), ).toBe("skip"); }); @@ -401,9 +405,11 @@ describe("app browser entry state helpers", () => { it("traces stale pending commits with compact reason codes and structured fields", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 5, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 4, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("skip"); @@ -414,21 +420,39 @@ describe("app browser entry state helpers", () => { code: NavigationTraceReasonCodes.staleOperation, fields: { activeNavigationId: 5, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 4, + startedVisibleCommitVersion: 0, }, }, ], }); }); + it("treats a visible commit version mismatch as stale before root-boundary decisions", () => { + const decision = resolvePendingNavigationCommitDispositionDecision({ + activeNavigationId: 2, + currentVisibleCommitVersion: 1, + currentRootLayoutTreePath: "/(marketing)", + nextRootLayoutTreePath: "/(dashboard)", + startedNavigationId: 2, + startedVisibleCommitVersion: 0, + }); + + expect(decision.disposition).toBe("skip"); + expect(decision.trace.entries[0]?.code).toBe(NavigationTraceReasonCodes.staleOperation); + }); + it("traces root-boundary hard navigation decisions", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/(marketing)", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("hard-navigate"); @@ -437,9 +461,11 @@ describe("app browser entry state helpers", () => { code: NavigationTraceReasonCodes.rootBoundaryChanged, fields: { activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/(marketing)", nextRootLayoutTreePath: "/(dashboard)", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }, }, ]); @@ -448,9 +474,11 @@ describe("app browser entry state helpers", () => { it("traces unknown root-layout identity as a legacy soft-commit fallback", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: null, startedNavigationId: 2, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("dispatch"); @@ -460,9 +488,11 @@ describe("app browser entry state helpers", () => { it("traces matching root-layout dispatches as current commits", () => { const decision = resolvePendingNavigationCommitDispositionDecision({ activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }); expect(decision.disposition).toBe("dispatch"); @@ -471,9 +501,11 @@ describe("app browser entry state helpers", () => { code: NavigationTraceReasonCodes.commitCurrent, fields: { activeNavigationId: 2, + currentVisibleCommitVersion: 0, currentRootLayoutTreePath: "/", nextRootLayoutTreePath: "/", startedNavigationId: 2, + startedVisibleCommitVersion: 0, }, }, ]); @@ -1020,6 +1052,122 @@ describe("app browser navigation controller", () => { } }); + it("does not let older same-URL server action payloads overwrite newer visible commits", async () => { + const initialState = createState({ + rootLayoutTreePath: "/", + routeId: "route:/settings", + navigationSnapshot: createClientNavigationRenderSnapshot("https://example.com/settings", { + tab: "profile", + }), + }); + const { controller, detach, stateRef } = createControllerHarness(initialState); + const { assign } = stubWindow("https://example.com/settings"); + let resolveOlderPayload!: (elements: AppElements) => void; + const olderPayload = new Promise((resolve) => { + resolveOlderPayload = resolve; + }); + + try { + const olderResult = controller.commitSameUrlNavigatePayload( + olderPayload, + stateRef.current.navigationSnapshot, + { + data: "older-action-result", + ok: true, + }, + ); + + const newerResult = await controller.commitSameUrlNavigatePayload( + Promise.resolve( + createResolvedElements("route:/settings/newer", "/", null, { + "page:/settings/newer": React.createElement("main", null, "newer"), + }), + ), + stateRef.current.navigationSnapshot, + { + data: "newer-action-result", + ok: true, + }, + ); + + expect(newerResult).toBe("newer-action-result"); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + + resolveOlderPayload( + createResolvedElements("route:/settings/older", "/", null, { + "page:/settings/older": React.createElement("main", null, "older"), + }), + ); + + await expect(olderResult).resolves.toBe("older-action-result"); + expect(assign).not.toHaveBeenCalled(); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + expect(stateRef.current.activeOperation).toMatchObject({ + lane: "server-action", + startedVisibleCommitVersion: 0, + state: "committed", + visibleCommitVersion: 1, + }); + } finally { + detach(); + } + }); + + it("uses the server-action initiation state when the response is processed after a newer commit", async () => { + const initialState = createState({ + rootLayoutTreePath: "/", + routeId: "route:/settings", + navigationSnapshot: createClientNavigationRenderSnapshot("https://example.com/settings", { + tab: "profile", + }), + }); + const { controller, detach, stateRef } = createControllerHarness(initialState); + const { assign } = stubWindow("https://example.com/settings"); + const actionInitiationState = stateRef.current; + + try { + const newerResult = await controller.commitSameUrlNavigatePayload( + Promise.resolve( + createResolvedElements("route:/settings/newer", "/", null, { + "page:/settings/newer": React.createElement("main", null, "newer"), + }), + ), + stateRef.current.navigationSnapshot, + { + data: "newer-action-result", + ok: true, + }, + ); + + expect(newerResult).toBe("newer-action-result"); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + + const olderResult = await controller.commitSameUrlNavigatePayload( + Promise.resolve( + createResolvedElements("route:/settings/older", "/", null, { + "page:/settings/older": React.createElement("main", null, "older"), + }), + ), + actionInitiationState.navigationSnapshot, + { + data: "older-action-result", + ok: true, + }, + actionInitiationState, + ); + + expect(olderResult).toBe("older-action-result"); + expect(assign).not.toHaveBeenCalled(); + expect(stateRef.current.routeId).toBe("route:/settings/newer"); + expect(stateRef.current.visibleCommitVersion).toBe(1); + } finally { + detach(); + } + }); + it("hard-navigates same-URL server action payloads when the root layout changes", async () => { const initialState = createState({ rootLayoutTreePath: "/(marketing)",