[PR #930] [MERGED] fix(app-router): keep ownerless URL commits from releasing navigation snapshots #957

New issue

Closed

opened 2026-05-06 13:11:14 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:14 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/930
Author: @NathanDrake2406
Created: 4/28/2026
Status: ✅ Merged
Merged: 4/28/2026
Merged by: @james-elicx

Base: main ← Head: nathan/fix-snapshot-ownership

📝 Commits (1)

8684250 fix(app-router): keep ownerless URL commits from releasing snapshots

📊 Changes

3 files changed (+104 additions, -10 deletions)

View changed files

📝 packages/vinext/src/server/app-browser-entry.ts (+2 -2)
📝 packages/vinext/src/shims/navigation.ts (+16 -8)
📝 tests/shims.test.ts (+86 -0)

📄 Description

What changed

Ownerless client URL sync commits no longer release an active App Router navigation render snapshot.

Why

commitClientNavigationState(undefined) covered both ownerless URL syncs and superseded-transition cleanup. Hash-only navigation and patched history.pushState / history.replaceState could therefore consume a snapshot owned by an in-flight navigation. Once consumed, usePathname() and useSearchParams() stopped reading the pending render snapshot and could flicker back to stale committed URL state mid-transition.

Approach

commitClientNavigationState now releases a snapshot only when called by a navigation owner (navId) or when a superseded App Router cleanup path explicitly opts into snapshot release. Ownerless URL sync paths still update committed pathname/search state and notify subscribers, but they do not balance activateNavigationSnapshot().

The App Router superseded cleanup sites pass { releaseSnapshot: true } while keeping navId undefined so they continue to avoid clearing pendingPathname owned by a newer navigation.

Validation

vp test run tests/shims.test.ts -t "keeps pending render snapshot active"
vp test run tests/app-browser-entry.test.ts
vp test run tests/shims.test.ts
vp check

Risks / follow-ups

The main risk is future superseded-transition cleanup code forgetting to opt into releaseSnapshot. The current App Router cleanup paths are explicit, and ownerless URL sync remains intentionally non-releasing.

Next.js references

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/930 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 4/28/2026 **Status:** ✅ Merged **Merged:** 4/28/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `nathan/fix-snapshot-ownership` --- ### 📝 Commits (1) - [`8684250`](https://github.com/cloudflare/vinext/commit/8684250e0a431b8364aeee8bfff1549561108ea2) fix(app-router): keep ownerless URL commits from releasing snapshots ### 📊 Changes **3 files changed** (+104 additions, -10 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/app-browser-entry.ts` (+2 -2) 📝 `packages/vinext/src/shims/navigation.ts` (+16 -8) 📝 `tests/shims.test.ts` (+86 -0) </details> ### 📄 Description ## What changed Ownerless client URL sync commits no longer release an active App Router navigation render snapshot. ## Why `commitClientNavigationState(undefined)` covered both ownerless URL syncs and superseded-transition cleanup. Hash-only navigation and patched `history.pushState` / `history.replaceState` could therefore consume a snapshot owned by an in-flight navigation. Once consumed, `usePathname()` and `useSearchParams()` stopped reading the pending render snapshot and could flicker back to stale committed URL state mid-transition. ## Approach `commitClientNavigationState` now releases a snapshot only when called by a navigation owner (`navId`) or when a superseded App Router cleanup path explicitly opts into snapshot release. Ownerless URL sync paths still update committed pathname/search state and notify subscribers, but they do not balance `activateNavigationSnapshot()`. The App Router superseded cleanup sites pass `{ releaseSnapshot: true }` while keeping `navId` undefined so they continue to avoid clearing `pendingPathname` owned by a newer navigation. ## Validation - `vp test run tests/shims.test.ts -t "keeps pending render snapshot active"` - `vp test run tests/app-browser-entry.test.ts` - `vp test run tests/shims.test.ts` - `vp check` ## Risks / follow-ups The main risk is future superseded-transition cleanup code forgetting to opt into `releaseSnapshot`. The current App Router cleanup paths are explicit, and ownerless URL sync remains intentionally non-releasing. ## Next.js references - [App Router pushState/replaceState restore path](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/app-router.tsx#L303-L372) - [App Router shallow routing hash navigation coverage](https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/shallow-routing/shallow-routing.test.ts#L450-L462) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>