useSearchParams Bug Fix | Do not regenerate the setSearchParams function when the searchParams change

[rwilliams3088]

I was encountering re-rendering errors in React and narrowed down the root cause to the useSearchParams() function. It turns out that the present implementation will unnecessarily regenerate the setSearchParams function every time the searchParams reference changes. If you are passing around the setSearchParams function to child components, you will find that the child components keeps getting re-rendered whenever the searchParams change - even if the child component isn't dependent upon the searchParams.

Example:

const Child = (props: ChildProps) => {
   const { setSearchParams } = props;
   ...
   // The child will be re-rendered and this effect will be triggered everytime you click the button w/o the bug fix
   useEffect(() => { console.log(`setSearchParams ref update!`) }, [setSearchParams]);
   ...
   return (<Button onClick={() => setSearchParams(...)}>Click Me!</Button>);
}

const Parent = () => {
   const [searchParams, setSearchParams] = useSearchParams();
   ...
   return (<Child setSearchParams={setSearchParams} ... />);
}

[remix-cla-bot[bot]]

Hi @rwilliams3088,

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 CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: b93f93d204fe3518e09d708020cb708b1d9aea3a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[remix-cla-bot[bot]]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[pongells]

can this be merged?

[IanVS]

It would be nice to hear something from maintainers, whether this is being considered or not.

[stasgavrylov]

@rwilliams3088 Could you please fix the tests so that your fix becomes ready to merge? It would be super useful. Thank you for your work.

[rwilliams3088]

@rwilliams3088 Could you please fix the tests so that your fix becomes ready to merge? It would be super useful. Thank you for your work.

If you pull it down and run it, you'll find all unit tests are passing. The issue on github is a memory issue due to whatever configuration they have setup for running the unit tests on here. However, since it has been a bit, I've gone ahead and update by branch with the latest. Maybe they've fixed their test config by now.

[684efs3]

Will this merged soon?

[rwilliams3088]

Will this merged soon?

I just submitted some revisions based upon the feedback above; waiting for the updates to be reviewed.

[684efs3]

Thank you!! :-)

[684efs3]

Who needs to review this? Is it @Brendonovich?

[barbalex]

Please merge this or solve the issue in another way. When keeping searchparams up to date on every navigation, every navigation leads to ALL components rerendering.

That is a no go. We are seriously considering migrating to tanstack router.

[NicoAdrian]

@rwilliams3088 any update ?

[rwilliams3088]

@NicoAdrian I only created the PR; it's up to whoever is maintaining this project to review and merge it. I have no clue what's going on behind the scenes. I'd have thought by now there'd have been some kind of response from the react-router team....

[NicoAdrian]

@brookslybrand Hello, any chance for this PR to be merged ?