feat(shims): add useUntrackedPathname hook for error boundary parity

[NathanDrake2406]

Summary

Next.js upstream provides a useUntrackedPathname() hook (in packages/next/src/client/components/navigation-untracked.ts) that is used by the unstable_catchError error boundary. Unlike usePathname(), useUntrackedPathname():

• Does not register the pathname as a tracked render dependency, avoiding unnecessary re-renders of the error boundary
• Returns the pending URL during an active navigation transition, then falls back to the committed URL so user pushState/replaceState changes are immediately reflected

What changed

1. Added useUntrackedPathname() to packages/vinext/src/shims/internal/navigation-untracked.ts (internal module, not exported from public next/navigation)

   • Server: returns / when no navigation context is available (the client will hydrate with the real value). Returns null only for missing-params shell prerenders — vinext does not yet implement fallback-route-param detection, so this path is not currently reachable.
   • Client: prefers the render snapshot only during an active navigation (navigationSnapshotActiveCount > 0), matching the existing invariant used by usePathname(), useSearchParams(), and useParams(). After commit, falls back to the cached pathname.

2. Switched error.tsx to import and use useUntrackedPathname() instead of usePathname() in the unstable_catchError wrapper.

3. Added tests covering:

   • Server-side / fallback when no context is set
   • Server-side pathname retrieval from navigation context
   • Client render snapshot preference during active navigation
   • Client committed pathname when provider snapshot is present but inactive (post-commit)
   • Pages Router pathname fallback
   • getDerivedStateFromProps boundary contract: clears errors on pathname changes, retains them when pathname stays the same

References

• Next.js source: https://github.com/vercel/next.js/blob/v16.2.6/packages/next/src/client/components/navigation-untracked.ts
• Next.js catch-error.tsx: https://github.com/vercel/next.js/blob/v16.2.6/packages/next/src/client/components/catch-error.tsx
• Identified during review of PR fix(app-router): preserve unstable_catchError boundaries #1906

Closes #1920

[pkg-pr-new]

Open in StackBlitz

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

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

commit: 0102e7b

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: useUntrackedPathname for error boundary parity

Solid, well-scoped change. I verified parity against Next.js v16.2.6's navigation-untracked.ts, ran vp check on the four changed source files (clean), and ran the full tests/shims.test.ts suite (1055 passed). Findings below are minor / non-blocking.

Verified correct

• No conditional-hook bug. isServer is a module-level constant, so the server branch (no hooks) and client branch (always one useContext via useClientNavigationRenderSnapshot) each keep a stable hook count per environment — same invariant Next.js relies on (hasFallbackRouteParams() always returns false on the client, so useContext is unconditional there).
• Active-navigation snapshot gate matches usePathname/useSearchParams/useParams (navigationSnapshotActiveCount > 0), so the hook returns the pending URL mid-transition and the committed cachedPathname post-commit. This is the right behavior for the getDerivedStateFromProps reset contract.
• getNavigationContext() === _getServerContext(), so swapping to the public export preserves the server path used by usePathname.
• Accessor refactor is sound. The new pages-router-accessor.ts uses the same Symbol.for("vinext.navigation.pagesNavigationContextAccessor") key registered in router.ts:2489, and navigation.ts keeps all call sites via the aliased import. Type shapes (PagesNavigationContext vs PagesNavigationContextShape) are structurally identical.
• Boundary contract tests are accurate — null→real pathname clears the error per _CatchError.getDerivedStateFromProps (error.tsx:172).

Minor suggestions (non-blocking)

1. useClientNavigationRenderSnapshot is now duplicated verbatim between navigation.ts (module-private, line 1330) and the new module. Two copies of the same hook-internals is a drift risk if the snapshot-context handling ever changes. Consider exporting the existing one from navigation.ts and importing it here instead.
2. The doc comment's "/" server fallback is an intentional divergence from Next.js (whose useContext(PathnameContext) default is null). It's consistent with vinext's own usePathname, so it's fine — just worth a one-line note that this is deliberate parity-with-usePathname rather than parity-with-Next.js, to avoid a future reader "fixing" it.

No correctness or security concerns. LGTM once you've considered the dedup suggestion.

[ask-bonk]

Review posted successfully to PR #1933.

Summary

I reviewed PR #1933 (feat(shims): add useUntrackedPathname hook for error boundary parity) and posted a COMMENT review. My verification:

Validated:

• Compared against Next.js v16.2.6's navigation-untracked.ts source
• Ran vp check on all 4 changed source files — clean (format + lint + types)
• Ran full tests/shims.test.ts — 1055 passed (including the 5 new useUntrackedPathname tests and the getDerivedStateFromProps boundary test)

Correctness confirmed:

• No conditional-hook ordering bug (isServer is module-level constant; client always calls exactly one useContext)
• Active-navigation snapshot gate (navigationSnapshotActiveCount > 0) matches the existing usePathname/useSearchParams/useParams invariant
• getNavigationContext() is equivalent to the internal _getServerContext() used by usePathname
• The pages-router-accessor.ts extraction uses the identical Symbol.for key registered in router.ts, and structural types match
• Boundary contract tests accurately reflect the getDerivedStateFromProps reset logic

Two minor, non-blocking notes:

1. useClientNavigationRenderSnapshot is duplicated between navigation.ts and the new module — a drift risk; could export and reuse the existing one.
2. The server / fallback is an intentional divergence from Next.js (null context default), consistent with vinext's usePathname — worth a one-line comment to prevent a future "fix."

No correctness or security concerns found.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: useUntrackedPathname for error boundary parity

Verified the change against Next.js v16.2.6's navigation-untracked.ts, ran vp check on all four changed source files (clean), and ran the new tests (useUntrackedPathname + getDerivedStateFromProps) — all green.

Verified correct

• Hook-count stability. isServer is a module-level constant; the client branch always calls exactly one hook (useClientNavigationRenderSnapshot → React.useContext), the server branch calls none. Each environment keeps a stable hook count, matching the upstream invariant.
• No useSyncExternalStore subscription. This is the whole point of the hook (avoiding re-renders on navigation), and the implementation correctly omits it while still returning a pathname that updates on navigation.
• Active-navigation gate (navigationSnapshotActiveCount > 0) matches the existing usePathname/useSearchParams/useParams invariant — returns pending URL mid-transition, committed pathname post-commit. This is exactly what the getDerivedStateFromProps reset contract needs.
• Symbol parity. pages-router-accessor.ts uses the same Symbol.for("vinext.navigation.pagesNavigationContextAccessor") registered in router.ts:2486, and the extracted type is structurally identical to the old inline one.
• getDerivedStateFromProps contract tests accurately reflect error.tsx:172 (clear on pathname change, retain on same pathname, clear on null→real).

Note on prior review

The earlier ask-bonk review flagged useClientNavigationRenderSnapshot as "duplicated verbatim" — that concern is now resolved. The PR exports the existing function from navigation.ts:1331 and imports it in the new module; there is only one definition. No action needed.

Minor (non-blocking)

One small redundancy in the client fall-through path — see inline comment. Not a correctness issue.

No correctness or security concerns. LGTM.

[ask-bonk]

Minor: the explicit Pages-context check here is redundant. getPathnameSnapshot() (lines 22-26) already calls getPagesNavigationContext() first and returns pagesCtx.pathname when present, so lines 64-65 duplicate that work. You can collapse to a single call:

Suggested change

	const pagesCtx = getPagesNavigationContext();
	if (pagesCtx) return pagesCtx.pathname;
	return getPathnameSnapshot();
	return getPathnameSnapshot();

Harmless either way (the extra lookup is cheap and side-effect-free), but removing it keeps this hook's client fall-through aligned with usePathname, which relies on getPathnameSnapshot alone for the same logic.

[ask-bonk]

Review posted successfully to PR #1933.

Summary

I reviewed PR #1933 (feat(shims): add useUntrackedPathname hook for error boundary parity) and posted a COMMENT review with one inline suggestion.

Verification performed:

• Compared against Next.js v16.2.6's navigation-untracked.ts source (fetched directly)
• Ran vp check on all 4 changed source files — clean (format + lint + types)
• Ran the new useUntrackedPathname tests (5 passed) and the getDerivedStateFromProps boundary test (passed)

Correctness confirmed:

• Hook-count stability is sound — isServer is a module-level constant, client always calls exactly one hook, server calls none
• The hook correctly omits useSyncExternalStore (the whole point — avoids re-render subscription)
• Active-navigation gate (navigationSnapshotActiveCount > 0) matches the existing usePathname/useSearchParams/useParams invariant
• Symbol parity verified between pages-router-accessor.ts and router.ts:2486
• Boundary contract tests accurately reflect error.tsx:172

Notable finding — prior review was stale: The earlier ask-bonk review flagged useClientNavigationRenderSnapshot as "duplicated verbatim." That is no longer true — the PR exports the existing function from navigation.ts:1331 and imports it; there's only one definition. I noted this in the review so it isn't acted on unnecessarily.

One minor, non-blocking suggestion: Lines 64-65 of navigation-untracked.ts redundantly check the Pages context, which getPathnameSnapshot() (line 66) already does. I posted an inline suggestion to collapse it. Harmless either way.

No correctness or security concerns. LGTM.

github run