useSearchParams no longer passes pending updates to multiple calls of setSearchParams function callback

[mkartchner994]

Reproduction

https://codesandbox.io/p/devbox/nervous-albattani-wmmls2

If you type in the search box, only the page param gets added to the url. Once downgraded to v7.6.3, the search param also gets added to the url.

System Info

React Router >=v7.7.0

Used Package Manager

npm

Expected Behavior

The function callback no longer passes the pending URLSearchParams to the next setSearchParams call if they happened one after another. For instance:

function useFilter() {
  const [searchParams, setSearchParams] = useSearchParams();

  const resetPage = useCallback(() => {
    setSearchParams((params) => {
      params.set("page", "1");
      return params;
    });
  }, [setSearchParams]);

  const updateSearch = useCallback((value) => {
    setSearchParams((params) => {
      params.set("search", value);
      return params;
    });
    // -------- This resetPage call will end up overriding setting search
    // because the next function callback no longer gets the
    // pending params from the last function callback above.
    // This worked prior to v7.7.0
    resetPage();
  }, [resetPage, setSearchParams]);

  return {
    updateSearch,
    resetPage,
  };
}

This example is a little contrived but we have some complex filtering logic that uses similar patterns.

Actual Behavior

The pending updates are not passed to the next setSearchParams call so previous calls get overridden with whatever is called last.

The react-router documentations says setSearchParams "also supports a function callback like React's setState". And React's setState function callback does work in this way where multiple callback calls get put in a queue and then called in the same order to calculate the final state.

[vladyslavgeyna]

I can reproduce the same issue with react-router version 7.7.0.

I have multiple setSearchParams calls one after another, like this:

const updateSearchParam = (param: string, value: string) =>
  setSearchParams((searchParams) => {
    searchParams.set(param, value);
    return searchParams;
  });

In the code below, both if conditions may be true:

onBlur={([updatedStartDate, updatedEndDate]) => {
  if (updatedStartDate && updatedStartDate !== startDate) {
    updateSearchParam(
      START_DATE_SEARCH_PARAM,
      dayjs(updatedStartDate).format(SEARCH_PARAMS_DATE_FORMAT)
    );
  }

  if (updatedEndDate && updatedEndDate !== endDate) {
    updateSearchParam(
      END_DATE_SEARCH_PARAM,
      dayjs(updatedEndDate).format(SEARCH_PARAMS_DATE_FORMAT)
    );
  }
}}

If both conditions are true, only END_DATE_SEARCH_PARAM gets set in the URL. START_DATE_SEARCH_PARAM is ignored — even though when I log inside the setSearchParams callback, I can see both being applied.

If I comment out the updatedEndDate block:

// if (updatedEndDate && updatedEndDate !== endDate) {
//   updateSearchParam(
//     END_DATE_SEARCH_PARAM,
//     dayjs(updatedEndDate).format(SEARCH_PARAMS_DATE_FORMAT)
//   );
// }

Then START_DATE_SEARCH_PARAM is set successfully.

It seems that each setSearchParams call doesn't account for pending updates from previous calls, which worked correctly prior to v7.7.0.

[timdorr]

#14041 (comment)

[mkartchner994]

@timdorr I don't believe this is a duplicate. The issue in #14041 (comment) appeared to be a race condition in effects. This issue was opened to address that pending updates are not passed to repeated calls of setSearchParams. This is a change from how it was working previously and does not act like React's setState like the react router documentation states. If this is how it should work going forward, I feel it would be best to have the documentation updated to not reference React's setState at all since they would no longer be similar. But also, if it is not suppose to work like this anymore, the function callback seems like an irrelevant pattern because it provides no benefits over passing the arg directly to the setSearchParams function.

I am happy to help look into this and try and create a PR if this is something the react-router team feels like should be addressed.

[timdorr]

The referenced PR (#12784) does apply here because the fix was to instantiate a new URLSearchParams for each callback of setSearchParams. That means that the mutation of the internal searchParams that used to work no longer does. This is an intentional change and fixes the prior buggy behavior (which the referenced comment states will be out-of-sync with the URL's search params).

The only claim that the docs make is that you can use a callback form, if preferred, not that it utilizes the same behavior. The fact it mirrored that behavior is unintentional.

The best fix is to rely on useState, since that has the behaviors you want. You can add a useEffect to watch for changes to a state value representing desired searchParams value, and calls one single setSearchParams to ensure the update is synchronous and non-overlapping.

[mkartchner994]

@timdorr I don't agree the documentation only claims the callback form is another option. Especially since it links directly to the react.dev setState documentation where in that section it states "React will calculate the next state by applying all of the queued updaters to the previous state". This makes it seem like the callback function works in this same way since it links directly to it and says "It also supports a function callback like React's setState". In that way if feels reasonable to say it should work like React's setState callback. Part of why I mentioned if the callback is just another option, it would be better not to mention or link to React's setState at all since they are not really similar.

I can understand if you don't agree.

We've already gone through and made the updates using useState as you've mentioned and things are working decent for us. It might be beneficial to add the useState pattern like you referenced here in the useSearchParams documentation as well. I'm happy to contribute a PR for something like that if interested.

Thanks for the feedback and taking the time to respond.

[timdorr]

I think adjusting the documentation is the best course of action here. The old behavior is unintentional, so reverting back to it would be a regression. Instead clarifying the differences with setState and giving an example for how to deal with multiple updates per render would be helpful to everyone.

[brophdawg11]

I agree - the callback syntax was a community PR (#8909 via #8955) back in 2022, but there was never any intentional "queueing" of updates in the hook - nor is that something I think we should try to replicate in RR code. I agree the docs would be best updated to specify that the queueing aspect of the callback form of setState is not supported.

@mkartchner994 Do you have an example of how you're using setState? If so we can include that in the docs update as well.

As an aside, we're planning to consolidate a bunch of hooks down into one in the near future and the setSearchParams setter will be going away in favor of direct navigations like navigate("?" + searchParams.toString()). That should help make things much clearer and avoid confusion of what that setter does internally IMO.

[mkartchner994]

@brophdawg11 What we ended up doing is wrapping the useSearchParams hook with our own useSearchParams hook that looks like this:

import { useCallback, useEffect, useRef } from 'react';
import { useSearchParams as useReactRouterSearchParams } from 'react-router';

export function useSearchParams() {
  const [reactRouterSearchParams, reactRouterSetSearchParams] = useReactRouterSearchParams();
  const spString = reactRouterSearchParams.toString();
  const searchParamsRef = useRef(new URLSearchParams(spString));
  const replaceRef = useRef(false);

  const setSearchParams = useCallback(
    (
      updater: (currentSearchParams: URLSearchParams) => URLSearchParams,
      options?: { replace?: boolean },
    ) => {
      searchParamsRef.current = updater(searchParamsRef.current);
      if (options?.replace) {
        replaceRef.current = true;
      }
      reactRouterSetSearchParams(searchParamsRef.current, {
        replace: replaceRef.current,
      });
    },
    [reactRouterSetSearchParams],
  );

  useEffect(() => {
    searchParamsRef.current = new URLSearchParams(spString);
    replaceRef.current = false;
  }, [spString]);

  return [reactRouterSearchParams, setSearchParams] as const;
}

It doesn't ended up using setState but we liked this approach because it allowed us to only need to update our import statements and not the call signatures. This also keeps react-router as the source of truth for the search param values in the return still (reactRouterSearchParams). Not sure if this is concurrent friendly but it fit our use case for what we are currently doing.

[brophdawg11]

ah ok thanks. I updated the docs with a note on this in a3b5b95