fix(react-router): make setSearchParams reference stable across renders#14785
fix(react-router): make setSearchParams reference stable across renders#14785DukeDeSouth wants to merge 2 commits intoremix-run:mainfrom
Conversation
`setSearchParams` was recreated on every render when search params changed because `searchParams` was listed as a `useCallback` dependency. This broke `useEffect` patterns where `setSearchParams` appeared in the dependency array, causing infinite re-renders or unnecessary effect re-runs. The fix stores `searchParams` in a ref that is updated via `useEffect` (not during render, to stay safe under concurrent rendering). The callback reads from the ref, so `searchParams` can be removed from the `useCallback` dependency array. This makes `setSearchParams` behave like React's `setState` — reference-stable across renders. The ref is updated in an effect rather than during render to comply with React's rules for concurrent mode, where render functions may be called multiple times before committing. This addresses the concern raised in the review of remix-run#10740. Fixes remix-run#9991 Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
Hi @DukeDeSouth, Welcome, and thank you for contributing to React Router! Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once. You may review the CLA and sign it by adding your name to contributors.yml. Once the CLA is signed, the If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run. Thanks! - The Remix team |
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳 |
1 participant
Human View
Summary
setSearchParamsreturned byuseSearchParams()is recreated every time search params change becausesearchParamsis in theuseCallbackdependency array. This causes:useEffect(() => { ... }, [setSearchParams])to re-run on every search params changesetSearchParamsas a prop to re-render unnecessarilyThis is a 3-year-old issue reported by @kentcdodds with 77 reactions and 33+ comments.
Solution
Store
searchParamsin a ref and update it viaReact.useEffect. The callback reads fromsearchParamsRef.currentinstead of the closure-capturedsearchParams, allowingsearchParamsto be removed from theuseCallbackdependency array.Why
useEffectand not render-time ref assignment?A previous PR (#10740) was reviewed by @Brendonovich who correctly pointed out that writing to
ref.currentduring render violates React's concurrent mode rules. This PR addresses that concern by updating the ref in an effect, which runs after React commits the render — making it safe under concurrent rendering.Why not
useLayoutEffect?useSearchParamscan run during SSR, whereuseLayoutEffectproduces warnings.useEffectis sufficient becausesetSearchParamsis never called between render and effect execution (it's only called from event handlers or other effects, both of which run after effects).Changes
packages/react-router/lib/dom/lib.tsx(+13/-3): AddsearchParamsRef+useEffect, update callback to read from ref, removesearchParamsfrom depspackages/react-router/__tests__/dom/search-params-test.tsx(+142/-0): Three new tests:setSearchParamsidentity is preserved across rendersuseEffect([setSearchParams])doesn't re-run on params changesetSearchParams(prev => ...)reads current params via refTest plan
setSearchParamsreference equality across rendersuseEffect([setSearchParams])fires once on mount onlyFixes #9991
AI View (DCCE Protocol v1.0)
Metadata
AI Contribution Summary
Verification Steps Performed
Human Review Guidance
searchParamsRef.current,ref.current,packages/react-router/lib/dom/lib.tsxMade with M7 Cursor