-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: Make setSearchParams stable? #9991
Comments
Also, calling |
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 |
This issue has been automatically marked stale because we haven't received a response from the original author in a while 🙈. This automation helps keep the issue tracker clean from issues that are not actionable. Please reach out if you have more information for us or you think this issue shouldn't be closed! 🙂 If you don't do so within 7 days, this issue will be automatically closed. |
Please don't close, I literally ran into this issue today 🙏🏼 |
We are still affected by this bug. This issue should not be closed until a fix is published. |
I use useCallback and it works! const updateSearchParams = useCallback(setSearchParams, []); Please kindly consider to make updateFunction a stable reference :) |
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 |
This is what I'm currently using as a workaround (following @zhengjitf stable function suggestion): import { useCallback, useEffect, useRef } from "react";
import { useSearchParams as useSearchParamsOrig } from "react-router-dom";
export function useSearchParams() {
const [searchParams, setSearchParams] = useSearchParamsOrig();
// Store a reference to the setter
const setSearchParamsRef = useRef(setSearchParams);
// Update the reference when the setter changes
useEffect(() => {
setSearchParamsRef.current = setSearchParams;
}, [setSearchParams]);
// Return a stable setter (which has no dep on searchParams)
const setSearchParamsStable = useCallback(
(...args: Parameters<typeof setSearchParams>) =>
setSearchParamsRef.current(...args),
[]
);
return { searchParams, setSearchParams: setSearchParamsStable };
} in case anybody is looking for the same workaround. It works for my use case, but I've not tested corner cases. |
@lucabergamini useEffect is no good, it would be best at the root off the component as the ref would be instantly updated instead of waiting for the render cycle to end in order to run the effects. |
Do you mean a |
@lucabergamini yes |
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. |
What is the progress on this? Will the fix be merged? |
I ran into this same issue today as well. |
it seems like a big foot gun, I noticed a bug in my app when calling |
any update? or does anyone know of an alternative to |
I encountered the same problem recently, I always assumed that the setter was stable |
Currently I see two issues with
Setters are usually imperative, not reactive. That is why React's |
What version of React Router are you using?
6.7.0
Steps to Reproduce
setSearchParams
in anything that needs a dependency array and add it to avoid issues.setSearchParams
in the dependency array triggers the useEffect any time the searchParams change (for example) to run againExpected Behavior
I expect to be able to use
setSearchParams
in auseEffect
and not have thatuseEffect
re-run just because search params changed.Actual Behavior
This is happening because
setSearchParams
allows you to pass a function callback which receives the currentsearchParams
sosetSearchParams
changes whenever thesearchParams
change. This is a nice API, but leads to unexpected re-runs of things usingsetSearchParams
.An alternative is to use a ref to keep track of the latest
searchParams
and pass thatref.current
instead sosearchParams
doesn't need to be listed in thenextInit
call. I can't think of a scenario where this would break anything.The code I'm talking about is here:
react-router/packages/react-router-dom/index.tsx
Lines 872 to 881 in 0ce0e4c
The text was updated successfully, but these errors were encountered: