fix(app-router): clear useLinkStatus pending after interrupted navigation (#1527)

[james-elicx]

Problem

useLinkStatus() is expected to return { pending: false } once a navigation settles. In vinext the pending flag stayed sticky (pending: 1) when a navigation was interrupted by a different navigation than the one the <Link> started — a second link click, a programmatic router.push/router.replace, a form submit, shallow routing via raw history.pushState, or browser back/forward. The <Link>'s own completion handler was the only thing that cleared its pending state, so an interrupting navigation left it stuck.

Surfaced by the Next.js Deploy Suite: test/e2e/use-link-status/index.test.ts (~3 failures). Part of #1328.

Fix

Introduce a small link-status pending registry (packages/vinext/src/shims/internal/link-status-registry.ts) that tracks the single <Link> driving the most recent App Router navigation, mirroring Next.js's linkForMostRecentNavigation / setLinkForCurrentNavigation adapted to vinext's per-<Link> React state model.

• A clicked <Link> registers its (mount-guarded) setPending setter via setLinkForCurrentNavigation, marking the navigation as link-initiated.
• navigateClientSide calls notifyLinkNavigationStart() at the start of every App Router navigation. A link-initiated navigation consumes its marker and stays pending; any other navigation resets the previously-tracked link to idle.
• The hook is also fired from the patched history.pushState/replaceState and a popstate listener so shallow routing and back/forward clear stale pending too.
• The hook is decoupled through navigation-runtime.ts (notifyLinkNavigationStart) to avoid a circular import between shims/link.tsx and shims/navigation.ts. Links drop their setter from the registry on unmount, so a later navigation never calls into an unmounted component.

Also hardened the Safari replaceState coalescing fallback in app-browser-entry.ts (syncCurrentHistoryStatePreviousNextUrl): it called the patched window.history.replaceState, which fires notifyLinkNavigationStart + commitClientNavigationState. On a URL-unchanged metadata sync that could clear an unrelated <Link>'s pending state a few ms early on Safari. The retry now goes through replaceHistoryStateWithoutNotify, so the browser sees the same native replaceState without the patched-path side effects.

Tests

tests/link-status-registry.test.ts (6 tests) reproduces the upstream scenarios at the registry level: link-initiated navigation stays pending; multi-click clears the previous link; router.push, shallow routing, and back clear sticky pending; unmounted/cleared links are never called; and unrelated pending links are left untouched.

Closes #1527

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1839

npm i https://pkg.pr.new/vinext@1839

commit: 530c47e

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[ask-bonk]

Review: clear useLinkStatus pending after interrupted navigation

Reviewed the registry, the navigateClientSide/history/popstate wiring, the <Link> integration, and the Safari replaceState hardening. The fix is well-scoped and the design is sound. Ran tests/link-status-registry.test.ts (6/6) and tests/link.test.ts (117/117) — all green. vp check passes on the new files.

What's correct and worth calling out

• Marker consumption relies on synchronous startTransition. In link.tsx the sequence is setLinkForCurrentNavigation(setter) → setPending(true) → startTransition(() => navigateClientSide(...)), and navigateClientSide consumes the marker as its very first synchronous statement (navigation.ts:1685). Because React runs the startTransition callback synchronously and navigateClientSide runs to its first await synchronously, the link-initiated marker is always set and consumed within the same click handler turn. I verified the tricky rapid-double-click-on-same-link case stays correct because each click re-sets the marker before its own notifyLinkNavigationStart consumes it. Good — but this is load-bearing and undocumented; see the suggestion below.

• App Router history commits won't prematurely clear the navigating link. The commit path in app-browser-entry.ts:373-375 uses pushHistoryStateWithoutNotify/replaceHistoryStateWithoutNotify, so committing a <Link> navigation does not fire notifyLinkNavigationStart. The patched pushState/replaceState only fire the hook when state.suppressUrlNotifyCount === 0, i.e. genuine shallow routing.

• Safari replaceState fallback fix is consistent. Routing the URL-unchanged metadata sync through replaceHistoryStateWithoutNotify (app-browser-entry.ts:640) matches the first-attempt path at line 629 and avoids the patched side effects. Correct.

• Unmount safety. clearLinkForCurrentNavigation on unmount + the mountedRef guard inside the stable setter mean a stale navigation never calls setState on an unmounted <Link>.

Suggestions (non-blocking)

1. PR body claims form submit is covered, but there's no direct path/test for it. The description lists "a form submit" as an interrupting navigation that clears pending. Server-action/form navigations only clear pending if they ultimately route through navigateClientSide or the patched pushState/replaceState. If forms commit history some other way, this case isn't actually covered. Worth confirming the form-submit path hits one of these three hooks, and ideally adding a registry-level test mirroring the upstream form-submit scenario (the PR ports the multi-click/router.push/shallow/back cases but not form submit).

2. Document the synchronous-startTransition dependency. The whole "keep link pending vs. reset" distinction hinges on setLinkForCurrentNavigation and the consuming notifyLinkNavigationStart running in the same synchronous turn. A one-line comment near setLinkForCurrentNavigation(setter) in link.tsx:980 noting that navigateClientSide must consume the marker synchronously (and why) would protect this invariant against a future refactor that defers the navigate call.

3. Two separate popstate listeners. navigation.ts now registers a dedicated notifyLinkNavigationStart popstate listener (2571) right before the existing Pages-Router scroll-restore listener (2579). Functionally fine since both are guarded by patchInstalled, but folding the call into the existing listener (or a comment explaining why it's intentionally separate) would reduce the chance someone later removes the "duplicate."

Parity note

Verified the four client entry points: the hook is fired from navigateClientSide (covers App Router programmatic + Link), patched pushState/replaceState (shallow routing), and popstate (back/forward). The registry is client-only and guarded by typeof window, so the RSC/SSR separate-module-graph gotcha doesn't apply. No prod-server/worker-entry changes needed since this is all browser-side runtime.

Overall: solid, behavior matches the Next.js linkForMostRecentNavigation model. The only thing I'd want resolved before merge is confirming the form-submit claim (suggestion 1).

[ask-bonk]

Review posted to PR #1839.

Summary

I reviewed the useLinkStatus pending-registry fix across all six changed files, traced the runtime wiring through the four client navigation paths, and ran the relevant tests (registry 6/6, link 117/117, vp check clean).

The fix is sound. Key findings:

• The marker consumption correctly relies on React's synchronous startTransition execution — setLinkForCurrentNavigation and the consuming notifyLinkNavigationStart always run in the same turn. I verified the rapid-double-click edge case stays correct.
• App Router history commits use the notify-suppressing *WithoutNotify helpers, so committing a <Link> navigation won't prematurely clear the navigating link's pending state.
• The Safari replaceState hardening is consistent with the existing first-attempt path.
• Unmount safety is handled via clearLinkForCurrentNavigation + mountedRef.

Non-blocking suggestions raised:

1. The PR body lists "form submit" as a covered interrupting navigation, but there's no direct path verification or test for it — worth confirming/adding.
2. Document the load-bearing synchronous-startTransition invariant near link.tsx:980.
3. Two separate popstate listeners now exist; consider folding or commenting.

The only item I'd want resolved before merge is confirming the form-submit claim.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: clear useLinkStatus pending after interrupted navigation

Independent re-review of the registry, the four navigation hook sites, the <Link> integration, and the Safari replaceState hardening. I traced every entry point, reasoned through the interrupted-navigation orderings, and ran the tests (tests/link-status-registry.test.ts 6/6, tests/link.test.ts 117/117, vp check clean on the new files).

The fix is correct and ready to merge. No blocking issues.

Verified correctness

• Marker can never get stuck true. setLinkForCurrentNavigation(setter) only runs inside the if (getNavigationRuntime()?.functions.navigate) block in link.tsx:980, immediately followed by startTransition(() => navigateClientSide(...)). navigateClientSide fires notifyLinkNavigationStart() as its first synchronous statement (navigation.ts:1685), before any early-return path (external/hash). Since React.startTransition runs its callback synchronously up to the first await, the link-initiated marker is always set and consumed in the same click turn. There is no code path that registers a link without then calling navigateClientSide.

• Interrupting-navigation orderings hold. Walked through Link A pending → Link B interrupts: setLinkForCurrentNavigation(B) resets A via the identity check (A !== B), B stays pending. When A's original promise later settles, A's .finally() calls clearLinkForCurrentNavigation(A), which no-ops because the tracker is now B (linkSetterForMostRecentNavigation === setter is false). Both B-settles-first and A-settles-first orderings are correct. The identity guard in clearLinkForCurrentNavigation is what makes stale completions safe.

• Faithful adaptation of Next.js links.ts. Compared against upstream setLinkForCurrentNavigation / linkForMostRecentNavigation / unmountLinkForCurrentNavigation. vinext correctly splits the single upstream function into the link-path setter plus a universal notifyLinkNavigationStart() (the link === null case), bridged by the currentNavigationIsLinkInitiated marker. Behaviorally equivalent. vinext resets the previous link outside a transition (upstream wraps in startTransition); harmless for an idle/urgent state update.

• Form submit IS covered (resolves the open question from the prior review). next/form GET navigation routes through navigateClientSide (form.tsx:453), which fires notifyLinkNavigationStart(). So a form-submit interrupting a pending <Link> clears it. POST/server-action forms don't client-navigate, matching Next.js. The PR-body claim is accurate; the only gap is a missing dedicated registry test for it (non-blocking).

• No double-clear / no premature clear. App Router history commits use pushHistoryStateWithoutNotify/replaceHistoryStateWithoutNotify (app-browser-entry.ts:373-375), so committing a <Link> navigation never re-fires the hook. Back/forward in App Router goes through handlePopstate → the runtime navigate (its own impl, not navigateClientSide), so notifyLinkNavigationStart fires once from the navigation.ts popstate listener. The function is idempotent for the clearing case regardless.

• Safari fallback is consistent. Both attempts in syncCurrentHistoryStatePreviousNextUrl now use replaceHistoryStateWithoutNotify (lines 629 and 640), avoiding the patched-path side effects on a URL-unchanged metadata sync. Correct.

• Unmount safety. clearLinkForCurrentNavigation(setter) on unmount + the mountedRef guard inside the stable setPendingRef setter prevent calling setState on an unmounted <Link>. The registry test confirms a dropped setter is never invoked.

Suggestions (non-blocking, can land separately)

1. Add a registry-level form-submit test. The path is covered at runtime, but the test suite ports multi-click / router.push / shallow / back and not form submit. A 7th test mirroring the upstream form-submit scenario would lock in the claim.
2. Document the synchronous-startTransition invariant. A one-line comment near setLinkForCurrentNavigation(setter) in link.tsx:980 noting that navigateClientSide must consume the marker synchronously (and why) would protect this load-bearing assumption against a future refactor that defers the navigate call.
3. Two popstate listeners in navigation.ts (2571 and 2579). Functionally fine — both under the patchInstalled guard. Folding or a "intentionally separate" comment would reduce the chance someone deletes the apparent duplicate.

Overall: well-scoped, behavior matches the Next.js linkForMostRecentNavigation model, registry is client-only (typeof window guarded) so the RSC/SSR module-graph gotcha doesn't apply. LGTM.

[ask-bonk]

Review posted to PR #1839.

Summary

I independently re-reviewed the useLinkStatus pending-registry fix across all six changed files, traced every navigation hook site, reasoned through the interrupted-navigation orderings, and ran the tests (registry 6/6, link 117/117, vp check clean on new files).

The fix is correct and ready to merge — no blocking issues.

Key verifications:

• Marker can never get stuck true — registration only happens immediately before navigateClientSide, which consumes the marker as its first synchronous statement, all within the same startTransition turn.
• Interrupting-navigation orderings hold — the identity guard in clearLinkForCurrentNavigation makes stale completions safe in both settle orders.
• Faithful adaptation of Next.js's links.ts linkForMostRecentNavigation model.
• Resolved the open question from the prior review: form submit is covered — next/form GET navigation routes through navigateClientSide (form.tsx:453), which fires the hook. POST/server-action forms don't client-navigate, matching Next.js.
• No premature clear — App Router history commits use the notify-suppressing helpers; Safari fallback is consistent.
• Unmount safety confirmed via clearLinkForCurrentNavigation + mountedRef.

Non-blocking suggestions: add a registry-level form-submit test, document the load-bearing synchronous-startTransition invariant near link.tsx:980, and consider folding/commenting the two popstate listeners.

github run