fix(router): decode hash scroll targets

[NathanDrake2406]

Overview

Field	Details
Goal	Match Next.js hash scrolling for URI-encoded fragments and named anchors.
Core change	Centralize hash target resolution, then use it from next/navigation, next/router, and app browser popstate restoration.
Primary files	packages/vinext/src/shims/hash-scroll.ts, packages/vinext/src/shims/navigation.ts, packages/vinext/src/shims/router.ts, packages/vinext/src/server/app-browser-entry.ts, tests/shims.test.ts
Expected impact	Hash-only client navigation can now scroll to id="hello world" via #hello%20world and can fall back to legacy name anchors.

Why

Hash navigation is a browser-visible compatibility surface. Next.js resolves hash fragments by decoding the URI fragment, checking id, and then falling back to name; vinext only checked raw ids in the router shims, so encoded ids and named anchors were skipped despite the app browser popstate path already having the correct behavior.

Area	Principle / invariant	What this PR changes
Hash fragment decoding	URL hash fragments are encoded transport, not DOM ids.	Decodes one decodeURIComponent pass before DOM lookup.
DOM target lookup	Browser-compatible hash scrolling checks id first, then name.	Adds the document.getElementsByName() fallback to router-driven hash navigation.
Runtime ownership	Shared browser behavior should not drift across router surfaces.	Moves the already-correct app browser logic into a small shared client helper.

What changed

Scenario	Before	After
Router.push("#hello%20world") with id="hello world"	Looked for hello%20world, did not scroll.	Looks for hello world, scrolls the id target.
Router.push("#legacy-anchor") with <a name="legacy-anchor">	Never checked named anchors.	Checks name after id miss and scrolls the first match.
#top and empty fragment	Only the app browser popstate path handled this consistently.	Shared helper preserves browser-style top scrolling across the router shims.
Malformed percent escape	App browser path recovered by using the raw fragment.	Shared helper keeps that vinext recovery behavior.

Maintainer review path

1. Review packages/vinext/src/shims/hash-scroll.ts for the exact target resolution order and malformed-fragment recovery.
2. Review packages/vinext/src/shims/router.ts and packages/vinext/src/shims/navigation.ts to confirm all old raw-id call sites use the shared helper.
3. Review packages/vinext/src/server/app-browser-entry.ts to confirm popstate restoration keeps its requestAnimationFrame scheduling while delegating the target decision.
4. Review tests/shims.test.ts for the focused red/green coverage through the public Pages Router singleton.

Validation

• Red check before implementation: vp test run tests/shims.test.ts -t "hash-only pushes" failed because getElementById received hello%20world and getElementsByName was never called.
• vp test run tests/shims.test.ts -t "hash-only pushes" passed after the fix.
• vp test run tests/shims.test.ts tests/app-browser-entry.test.ts passed, 1003 tests.
• vp check passed formatting, lint, and type checks.
• Commit hook also reran checked-file formatting, lint/type checks, and knip.

Risk / compatibility

• Public API: no new public Next.js API surface.
• Runtime: browser-only hash scrolling behavior changes to match Next.js and browser anchor behavior more closely.
• Compatibility: intentional vinext-specific recovery is preserved for malformed percent escapes by attempting raw-fragment matching instead of throwing during navigation.
• RSC/App Router: app browser popstate still defers hash scrolling to the next animation frame.

References

Reference	Why it matters
Next.js Pages Router scrollToHash	Decodes the hash, handles #top, checks id, then falls back to name.
Next.js App Router navigation hash decoding	Decodes the hash fragment before storing scroll intent.
Next.js App Router hash DOM lookup	Resolves top, then id, then name in the scroll/focus handler.
Next.js Pages Router CJK hash tests	Confirms encoded/non-Latin hash anchors are expected to scroll.
Next.js Pages Router named-anchor test	Confirms name fallback is expected behavior.
Next.js App Router hash navigation tests	Confirms App Router hash scrolling and scroll={false} behavior.

[pkg-pr-new]

Open in StackBlitz

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

commit: 67ab06d

[james-elicx]

/bigbonk review

[ask-bonk]

Solid fix. The shared helper matches Next.js's scrollToHash behavior precisely (decode → #top/empty → getElementById → getElementsByName), the scroll: false guard that was missing on hash-only paths is now correctly applied, and the test coverage is thorough.

I verified:

• scrollToHashTarget in hash-scroll.ts matches the Next.js Pages Router scrollToHash: decode, handle "" and "top", check id, fall back to name.
• Next.js guards hash scrolling with if (scroll) in the hash-only branch (where scroll defaults to true from const { scroll = true } = options). The old vinext code unconditionally called scrollToHash — this PR correctly adds the scroll !== false guard to all hash-only paths.
• The app-browser-entry.ts refactor preserves the requestAnimationFrame scheduling via scrollToHashTargetOnNextFrame.
• The three new tests cover the key scenarios: URI-decoded id, name fallback, and scroll: false opt-out.

One minor observation inline, but not blocking.

[ask-bonk]

Nit: Next.js calls scrollIntoView() (no args) rather than scrollIntoView({ behavior: "auto" }). They're functionally equivalent per spec, so this is fine — just noting the difference in case someone ever diffs against Next.js source and wonders.

[ask-bonk]

Review submitted with approval. Here's a summary:

Approved — this is a clean, well-scoped fix that:

1. Centralizes hash scrolling into hash-scroll.ts with the correct resolution order matching Next.js: decode URI → handle "" / "top" → getElementById → getElementsByName fallback.

2. Fixes two bugs: encoded hash fragments (e.g., #hello%20world) now decode before DOM lookup, and name attribute anchors now work as a fallback — both matching Next.js behavior.

3. Fixes a third latent bug: hash-only navigation paths previously ignored scroll: false, which Next.js respects. All hash-only branches now guard with options?.scroll !== false.

4. Preserves the requestAnimationFrame scheduling in app-browser-entry.ts via the dedicated scrollToHashTargetOnNextFrame wrapper.

5. Has good test coverage: three focused tests for URI-decoded id targets, named anchor fallback, and scroll: false opt-out.

github run