react-router: [Bug]: Make setSearchParams stable?

What version of React Router are you using?

6.7.0

Steps to Reproduce

Use setSearchParams in anything that needs a dependency array and add it to avoid issues.
Notice that the presence of setSearchParams in the dependency array triggers the useEffect any time the searchParams change (for example) to run again

Expected Behavior

I expect to be able to use setSearchParams in a useEffect and not have that useEffect re-run just because search params changed.

Actual Behavior

This is happening because setSearchParams allows you to pass a function callback which receives the current searchParams so setSearchParams changes whenever the searchParams change. This is a nice API, but leads to unexpected re-runs of things using setSearchParams.

An alternative is to use a ref to keep track of the latest searchParams and pass that ref.current instead so searchParams doesn’t need to be listed in the nextInit call. I can’t think of a scenario where this would break anything.

The code I’m talking about is here:

https://github.com/remix-run/react-router/blob/0ce0e4c728129efe214521a22fb902fa652bac70/packages/react-router-dom/index.tsx#L872-L881

About this issue

Original URL
State: open
Created a year ago
Reactions: 44
Comments: 21 (2 by maintainers)

Most upvoted comments

I feel that all functions in hooks, especially those we’ll call in other hooks or as props to a component, are supposed to be stable. we just need to be able to get the latest states when these functions are called.

The following is a simple solution to create stable functions:

/* in a hook */

const fn = { /*... */ }

const fnRef = useRef(fn)
fnRef.current = fn
const stableFn = useCallback((...args) => fnRef.current(...args), [])

// then, we can use stableFn

zhengjitf on Feb 7, 2023

What is the progress on this? Will the fix be merged?

tengl on Oct 3, 2023

Currently I see two issues with setSearchParams.

It is not stable
It doesn’t pass the latest value in its callback function (see @maastrich’s comment). This can be worked-around with flushSync but that is a waste of renders.

Setters are usually imperative, not reactive. That is why React’s useState is a stable function and it allows us to pass a callback to ensure we are working with the latest data.

KurtGokhan on Mar 12, 2024

I ran into this same issue today - and since it was causing React rendering errors in the console, I decided to go ahead and put together a fix: https://github.com/remix-run/react-router/pull/10740

rwilliams3088 on Jul 27, 2023

Also, if multiple updates happen in the same render cycle, using the new value as a dependency of the callback causes it to no be provided within the prev param of the n+1 setSearchParams call

maastrich on Jun 1, 2023

Also, calling setSearchParams will re-render the whole route. It would be very helpful if at least there would be some sort of flag on each route config, similar to shouldRevalidate, that let’s you opt out of such re-renders.

max-pcentra on Jan 29, 2023

any update? or does anyone know of an alternative to react-router which has a stable solution?

finnhvman on Nov 27, 2023

I ran into this same issue today - and since it was causing React rendering errors in the console, I decided to go ahead and put together a fix: #10740

Thank you! Hope to see this merged soon!

I ran into the issue today, absolutely unexpected and seems very dangerous.

finnhvman on Aug 7, 2023

@lucabergamini yes

maastrich on Jun 15, 2023