[Docs]: Missing info for useSearchParams

[alesmenzel]

Describe what's incorrect/missing in the documentation

It would be beneficial to mention in the docs whether user of react-router can mutate the previous searchParams or they should create a new instance.

https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router-dom/index.tsx#L1467C7-L1467C33

const [searchParams, setSearchParams] = useSearchParams()

setSearchParams(searchParams => {
  searchParams.set('test', 'test')
  return searchParams
})

// versus

setSearchParams(searchParams => {
  const updated = new URLSearchParams(searchParams)
  updated.set('test', 'test')
  return updated
})

Since react-router does create a new URLSearchParams which is memorized by location.search https://github.com/remix-run/react-router/blob/main/packages/react-router-dom/dom.ts#L79, it seems it should be fine to just mutate the search params instead of creating new URLSearchParams. Can you please specify this in the docs.

[kailash360]

@alesmenzel Can I work on this?

[alesmenzel]

Hi @kailash360, feel free to work on this issue, but I think the maintainers should first give us their opinion though.

[yoganandaness]

@alesmenzel I think there is a real difference between the two.

setSearchParams(searchParams => { searchParams.set('test', 'test') return searchParams }) This mutates the searchParams directly, since set is a mutable function. As a result of this, before even returning the searchParams, the searchParams gets set with new value. This will result in the variable getting set before even the url changes takes place.

setSearchParams(searchParams => { const updated = new URLSearchParams(searchParams) updated.set('test', 'test') return updated })

This will not do any harm to the searchParams, since this was cloned into the variable updated.

This will really affect in a scenario where we do window.confirm. When clicking No, the url and searchParams variable should not change, but here the variable changes does when following the approach 1. Approach 2 is good, but cloning the searchParams is a super cumbersome task.

Do you agree? Correct me if i am wrong.

[ryanflorence]

I've updated the docs and I think it covers this question, lmk if not, thank you!

Note that searchParams is a stable reference, so you can reliably use it as a dependency in useEffect hooks.

However, this also means it's mutable. If you change the object without calling setSearchParams, its values will change between renders if some other state causes the component to re-render and URL will not reflect the values.

https://reactrouter.com/dev/api/hooks/useSearchParams

[alesmenzel]

Hi @ryanflorence thanks for clarifying it. Would you please 🙏 also consider adding info that users cannot perform multiple setSearchParams() calls in the same render cycle, e.g. defining 2 effects with setSearchParams.

Here is a contrived example for a table component

function TableComponent ({ query, sort }) {
  const [searchParams, setSearchParams] = useSearchParams();

  useEffect(() => {
    setSearchParams((searchParams) => searchParams.set("query", query))
  }, [setSearchParams, query])

  useEffect(() => {
    setSearchParams((searchParams) => searchParams.set("sort", sort))
  }, [setSearchParams, sort])

  return <Table>...</Table>
}

With multiple simultaneous calls, one of the setSearchParams calls will get ignored, because the searchParams of the second call won't include the first update as mentioned in https://github.com/remix-run/react-router/issues/12179

[alesmenzel]

Because the setSearchParams call is a navigate call under the hood as explained here - https://github.com/remix-run/react-router/issues/12179#issuecomment-2432781035