fix(router): share pages useRouter state through context

[NathanDrake2406]

Overview

Field	Details
Goal	Align Pages Router useRouter() with Next.js context semantics and avoid per-hook URL subscriptions.
Core change	Move the reactive Pages Router snapshot into wrapWithRouterContext()'s provider and make next/router consume RouterContext.
Main boundary	The Pages Router tree owns router state. Hook consumers read that state and do not install global listeners.
Primary files	packages/vinext/src/shims/router.ts, tests/shims.test.ts
Expected impact	Components using next/router and next/compat/router share one provider-backed router value during hydration, SSR, and client navigation.

Why

Pages Router correctness depends on a single mounted router context being the source of truth for route fields and navigation methods. Next.js implements next/router by reading RouterContext; vinext already mounted that context, but next/router bypassed it and created hook-local state plus a vinext:navigate listener per hook call.

Area	Principle / invariant	What this PR changes
next/router	useRouter() reads the mounted Pages Router context and throws when it is absent.	useRouter() now consumes RouterContext and uses the Next.js not-mounted error.
Router state ownership	The Pages Router tree owns the reactive URL snapshot.	wrapWithRouterContext() now returns a provider component that tracks the URL once.
Listener scaling	Hook call count should not scale global navigation listeners.	vinext:navigate is subscribed at the provider boundary, not in each hook invocation.

What changed

Scenario / surface	Before	After
Many components call next/router useRouter()	Each hook call created independent state and a vinext:navigate listener.	Hook calls read the same provider value.
useRouter() outside Pages Router context	Returned a synthetic router snapshot in some test paths.	Throws NextRouter was not mounted, matching Next.js.
next/compat/router	Consumed RouterContext, but next/router used a separate path.	Both surfaces share the same provider-backed router value.

Maintainer review path

1. packages/vinext/src/shims/router.ts
   • Review useRouter() for Next.js parity.
   • Review PagesRouterProvider ownership of vinext:navigate subscription and router value construction.
   • Review Router singleton methods to confirm command behavior remains unchanged.
2. tests/shims.test.ts
   • Review the new context, missing-context, and listener-scaling regression tests.
3. packages/vinext/src/entries/pages-client-entry.ts and packages/vinext/src/server/dev-server.ts
   • Comment-only updates that clarify both next/router and next/compat/router are covered by the wrapper.

Validation

• vp check
• vp test run tests/shims.test.ts tests/pages-router.test.ts -t "next/router withRouter HOC|Pages Router router helpers|next/compat/router"
• PLAYWRIGHT_PROJECT=pages-router pnpm run test:e2e -- tests/e2e/pages-router/navigation.spec.ts tests/e2e/pages-router/shallow-routing.spec.ts
• Commit hook also ran tests/entry-templates.test.ts and repository checks.

Risk / compatibility

• Public API: next/router now throws outside a mounted Pages Router context, matching Next.js. This may surface invalid unit-test setup that vinext previously tolerated.
• Runtime: shallow, hash, push, replace, and popstate navigation still use the existing singleton methods and event dispatches.
• Compatibility: next/compat/router behavior remains nullable outside context and provider-backed inside Pages Router.

Non-goals

• This does not change the Pages Router navigation algorithm or route event ordering.
• This does not make the Pages Router singleton itself a full Next.js Router class implementation.

References

Reference	Why it matters
Next.js next/router useRouter()	Shows useRouter() reading RouterContext and throwing when it is not mounted.
Next.js client RouterContext.Provider	Shows Pages Router app rendering with RouterContext.Provider.
Next.js RouterContext	Defines the shared Pages Router context used by next/router and next/compat/router.
Next.js next/compat/router	Confirms compat router also consumes RouterContext, returning null outside context.
Next.js NextRouter was not mounted docs	Documents the intended error when next/router is used outside a mounted Pages Router context.
Next.js useRouter docs	Documents the Pages Router object returned by useRouter() and withRouter().

[pkg-pr-new]

Open in StackBlitz

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

commit: 14b8cbf

[james-elicx]

/bigbonk review

[ask-bonk]

Clean refactor that aligns next/router with Next.js context semantics. The core change — making useRouter() consume RouterContext instead of managing hook-local state — is correct and well-motivated. The PagesRouterProvider lifts the reactive URL subscription to the provider boundary, eliminating O(n) listeners.

SSR path is safe: useState initializes from getPathnameAndQuery() which reads SSR context on the server, useEffect is a no-op during SSR (React skips effects during renderToString/renderToStaticMarkup), and useMemo computes the router value synchronously. All wrapWithRouterContext call sites (dev-server, prod-server entry, pages-client-entry, and the navigateClient function) continue to work because wrapWithRouterContext still wraps in the same context provider, just via a component now.

The deletion of the duplicated push/replace/back/reload/prefetch useCallback wrappers is a big win — these were exact copies of the Router singleton methods, and the hook-local setState calls they contained were redundant with the vinext:navigate event that the singleton methods already dispatch. Good cleanup.

Two minor observations below, neither blocking.

[ask-bonk]

Nit: the useEffect on line 670 unconditionally references window, which would throw if this component somehow rendered on the server outside React's renderer (e.g., if someone called the effect directly in a test). In practice this is fine because React skips useEffect during SSR, so the callback never runs. But a typeof window !== "undefined" guard inside the effect would make it defensive.

Not blocking — useEffect has been a server no-op since React 16 and that contract is stable.

[ask-bonk]

Minor: this React mock covers exactly the hooks PagesRouterProvider and useRouter use today (createContext, createElement, useContext, useState, useEffect, useMemo). If a future change adds a hook the mock doesn't cover, this test will fail with an opaque undefined is not a function error. Consider adding a brief comment listing the mocked hooks and why, so maintainers know the surface is intentionally minimal.

[ask-bonk]

Review posted and approved. Here's a summary of the review:

Approved with two non-blocking observations.

Key findings:

1. Correctness — The refactor correctly moves useRouter() from managing hook-local state + per-hook vinext:navigate listeners to consuming RouterContext via useContext. This matches how Next.js implements next/router (reading RouterContext and throwing when not mounted).

2. SSR safety — PagesRouterProvider is safe during SSR: useState initializes synchronously from getPathnameAndQuery() (which reads the SSR context on the server), useEffect is a no-op during SSR, and useMemo computes the router value synchronously.

3. Code deletion — The ~150 lines of deleted useCallback wrappers for push/replace/back/reload/prefetch were exact duplicates of the Router singleton methods. The hook-local setState calls they contained were redundant with the vinext:navigate event the singleton already dispatches. Good cleanup.

4. Dev/prod parity — All wrapWithRouterContext call sites (dev-server, prod-server entry, pages-client-entry, navigateClient) continue to work correctly.

5. Two minor nits posted as inline comments: (a) the useEffect in PagesRouterProvider references window without a guard, safe in practice but slightly fragile; (b) the React mock in the listener-scaling test could use a comment documenting which hooks it intentionally covers.

github run