fix: useRouter() returns stable singleton to prevent unnecessary re-renders#321
Conversation
…enders Next.js returns the same router object reference on every call to useRouter() from next/navigation. Our implementation was creating a new object literal on each call, which breaks components that rely on referential equality — useMemo/useEffect dependency arrays and React.memo bailouts would fire unnecessarily on every render. Hoist the router object to module level so useRouter() always returns the same reference. All methods close over module-level state and carry no per-render data, so this is safe. Adds two regression tests: one asserting reference equality across multiple calls, and one verifying the singleton exposes all expected navigation methods.
commit: |
|
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, correct change. The fix is straightforward and the reasoning is sound.
What changed: The router object literal that was previously created inside useRouter() on every call is hoisted to a module-level const _appRouter. The function now returns that singleton directly.
Why it's safe: Every method on the router object (push, replace, back, forward, refresh, prefetch) closes over module-level functions (navigateImpl, withBasePath, toRscUrl, etc.) and reads from module-level or window-level state at call time — none of them capture any per-render data. There's no reason to create a new object per call.
Why it matters: Components that put router in a dependency array (useMemo, useEffect, useCallback) or are wrapped in React.memo comparing props would trigger unnecessary re-renders on every parent render, since {} !== {}. This matches how Next.js handles it.
Tests are good — referential equality check (===) across three calls, plus a method existence check. All 568 tests pass.
No issues found.
|
PR approved. The change is clean and correct:
|
1 participant
Summary
Changes
Testing