[PR #929] [MERGED] fix(app-router): restore hash anchors on history traversal #956

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/929
Author: @NathanDrake2406
Created: 4/28/2026
Status: ✅ Merged
Merged: 4/28/2026
Merged by: @james-elicx

Base: main ← Head: nathan/fix-hash-popstate-scroll

📝 Commits (2)

fbc6fdf fix(app-router): restore hash anchors on history traversal
78ce7db test(app-router): cover hash scroll edge cases

📊 Changes

3 files changed (+161 additions, -0 deletions)

View changed files

📝 packages/vinext/src/server/app-browser-entry.ts (+30 -0)
➕ tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts (+93 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/hash-popstate-scroll/page.tsx (+38 -0)

📄 Description

What changed

App Router popstate scroll restoration now falls back to hash-anchor scrolling when the traversed history entry has no vinext numeric scroll state.

A focused App Router e2e fixture and regression tests cover vinext-managed hash-only Link entries through back and forward traversal, including decoded non-latin IDs, named-anchor fallback, and #top.

Why

vinext sets history.scrollRestoration to manual. Hash-only App Router navigations can push entries without __vinext_scrollY, so forward traversal to /path#anchor updated the URL but skipped the browser native anchor scroll and could leave the page at the top.

Approach

The popstate restoration path keeps saved numeric scroll state as the first-priority behavior. Only entries without vinext scroll state fall back to window.location.hash.

Hash fallback mirrors practical Next.js semantics: decode the fragment when possible, treat #top and empty fragments as top, prefer id lookup, then fall back to name lookup. Malformed encodings are left as-is instead of throwing. The fallback uses explicit auto scroll behavior to match the existing vinext hash-scroll helpers.

Validation

Added a failing Playwright regression first, then verified it passed after the fix: PLAYWRIGHT_PROJECT=app-router vp run test:e2e -- tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts
Expanded the same E2E coverage after review for decoded fragments, named anchors, and #top: PLAYWRIGHT_PROJECT=app-router vp run test:e2e -- tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts
Ran adjacent unit coverage: vp test run tests/app-browser-entry.test.ts
Ran repo checks: vp check
Rebuilt the vinext package for local e2e validation: vp run vinext#build

Risks / follow-ups

Numeric back and forward restoration intentionally remains higher priority than hash scrolling, so entries with saved __vinext_scrollY keep existing behavior.

This does not attempt to refactor the existing duplicate hash-scroll helpers between App Router browser entry and next/navigation; the change stays scoped to the App Router popstate gap.

Next.js references

App Router records hash scroll intent in focusAndScrollRef: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/segment-cache/navigation.ts#L596-L693
App Router resolves id and name hash targets in layout-router: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/layout-router.tsx#L122-L140
App Router consumes hash scroll after navigation commits: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/layout-router.tsx#L306-L313
App Router hash-scroll e2e coverage: https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/navigation/navigation.test.ts#L149-L211
Pages Router hash target semantics for #top, id, name, and decodeURIComponent: https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/router.ts#L2323-L2348

_{🔄 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/929 **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-hash-popstate-scroll` --- ### 📝 Commits (2) - [`fbc6fdf`](https://github.com/cloudflare/vinext/commit/fbc6fdf65c2623b0316ff0ef397db1517d38d062) fix(app-router): restore hash anchors on history traversal - [`78ce7db`](https://github.com/cloudflare/vinext/commit/78ce7dbc7103eb6cc06b38db055b93c2e2eb58cc) test(app-router): cover hash scroll edge cases ### 📊 Changes **3 files changed** (+161 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/app-browser-entry.ts` (+30 -0) ➕ `tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts` (+93 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/hash-popstate-scroll/page.tsx` (+38 -0) </details> ### 📄 Description ## What changed App Router popstate scroll restoration now falls back to hash-anchor scrolling when the traversed history entry has no vinext numeric scroll state. A focused App Router e2e fixture and regression tests cover vinext-managed hash-only Link entries through back and forward traversal, including decoded non-latin IDs, named-anchor fallback, and #top. ## Why vinext sets history.scrollRestoration to manual. Hash-only App Router navigations can push entries without __vinext_scrollY, so forward traversal to /path#anchor updated the URL but skipped the browser native anchor scroll and could leave the page at the top. ## Approach The popstate restoration path keeps saved numeric scroll state as the first-priority behavior. Only entries without vinext scroll state fall back to window.location.hash. Hash fallback mirrors practical Next.js semantics: decode the fragment when possible, treat #top and empty fragments as top, prefer id lookup, then fall back to name lookup. Malformed encodings are left as-is instead of throwing. The fallback uses explicit auto scroll behavior to match the existing vinext hash-scroll helpers. ## Validation - Added a failing Playwright regression first, then verified it passed after the fix: PLAYWRIGHT_PROJECT=app-router vp run test:e2e -- tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts - Expanded the same E2E coverage after review for decoded fragments, named anchors, and #top: PLAYWRIGHT_PROJECT=app-router vp run test:e2e -- tests/e2e/app-router/nextjs-compat/hash-popstate-scroll.spec.ts - Ran adjacent unit coverage: vp test run tests/app-browser-entry.test.ts - Ran repo checks: vp check - Rebuilt the vinext package for local e2e validation: vp run vinext#build ## Risks / follow-ups Numeric back and forward restoration intentionally remains higher priority than hash scrolling, so entries with saved __vinext_scrollY keep existing behavior. This does not attempt to refactor the existing duplicate hash-scroll helpers between App Router browser entry and next/navigation; the change stays scoped to the App Router popstate gap. ## Next.js references - App Router records hash scroll intent in focusAndScrollRef: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/segment-cache/navigation.ts#L596-L693 - App Router resolves id and name hash targets in layout-router: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/layout-router.tsx#L122-L140 - App Router consumes hash scroll after navigation commits: https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/layout-router.tsx#L306-L313 - App Router hash-scroll e2e coverage: https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/navigation/navigation.test.ts#L149-L211 - Pages Router hash target semantics for #top, id, name, and decodeURIComponent: https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/router.ts#L2323-L2348 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>