Avoiding race conditions when updating search

[bensaufley]

I see #368 which may be related but I can't find anything that addresses simultaneous or near-simultaneous updates to the search. I'm probably going to end up implementing my own solution in my codebase in one way or another but I thought I'd bring it up here in case there's already thought on how this can be managed.

I have a function called useQueryParam that does a bunch of coercion etc but at its core it's basically:

const useQueryParam = (param: string) => {
  const [loc, navigate] = useLocation();
  const search = useSearch();

  const value = useMemo(() => new URLSearchParams(search).get(param), [search, param]);
  const setter = useEvent((newValue: string | null) => {
    const params = new URLSearchParams(search);
    if (setter === null) params.delete(param);
    else params.set(param);

    navigate(`${loc}${params.toString()}`);
  };

  return [value, setter] as const;
};

It basically behaves like useState but persists to the query string.

With the prior version of Wouter, I had just hooked directly into window.location.search inside the setter so the params were always the latest when setting.

But with the latest version, for consistency and better testability (JSDOM is notoriously … not useful) I wanted to migrate to useSearch, which can consume from the <Router> context which can be in-memory. But that means that if two setters execute simultaneously (say, setting a default value on page render, from different subcomponents – making it hard to batch them in other ways), they encounter a race condition where both setters are operating from the same value of search so the second one overwrites the changes of the first.

As I understand it, even though useEvent will use the values from the latest invocation of the hook, the hook will only be invoked on a re-render and re-render may not have triggered between the first call to navigate and the second reference to search; am I conceptualizing that right?

Does this problem explanation make sense? Again, I don't think this blocks me, because a) I just won't upgrade Wouter until I can sort this out and b) I have a number of possible homegrown solutions in mind; but I'm wondering if this is a solved problem and I missed it, or a problem that you're thinking about?

[bensaufley]

BTW I'm using Preact (annoyingly, with preact/compat); I don't think it matters, but just to be transparent.

[molefrog]

Hello, it took me some time to wrap my head around it, but I think I finally got the idea. A just wanted to confirm: is this something hypothetical or a real case that you were able to reproduce? Also, assuming that instead of calling 'navigate' you worked with context value (via state), wouldn't this also create race condition?

[molefrog]

If we have two components that set state to different values simultaneously, this might create some ambiguity in my opinion, because React doesn't guarantee the exact order of useEffect invocations (I suppose this is where the state is changed).

[bensaufley]

So yeah this is a real-life situation. Unfortunately, it's in a private repo so I can't share exactly the implementation I've gone with but basically, not worrying specifically about two things trying to set the same property but separate properties. E.g., one component gets and sets "name" from the query string and another gets and sets "email"; but by simply having a string coming from useSearch, there's a scenario where each of those components receives the same search value, adds its own update, and then one overwrites the other:

const search = useSearch();
// If an event like this is called from two different components near-enough
// that a rerender is not triggered, `search` will not have the other component's
// value on it, so `params.toString()` will omit that value
useEvent((email: string) => {
  const params = new URLSearchParams(search);
  params.set('email', email);
  navigate(loc + '?' + params.toString();
});

The way these race conditions are generally resolved in other [p]react hooks is by allowing for callbacks, such as in setState((prev) => ({ ...prev, email })). This allows for special handling (e.g., choosing not to overwrite email if it already exists on prev) as well as only modifying what needs modification (spreading the rest of prev).

What I've ended up doing is basically creating a wrapper context that just keeps a queue of updates to apply to the search string one at a time, popping from the queue on each render (applying from the queue causes a re-render since search is updated, so it will basically re-render until the queue is empty). It's messy and I'm not really a fan of it but because I can't hook into the raw "what is the value right now" while also using the Router context (rather than, e.g., directly referencing window.location.search, which is less easily testable), it's what I feel like I'm stuck with.

[molefrog]

I see, makes sense now. Have you tried wrapping navigate() in flushSync? This should (in theory) make React flush updates to the DOM before calling the next callback.

[bensaufley]

:( looks like preact's implementation is just a straight passthrough to the internal function.

I'm not sure it would solve this problem—because it seems like it's more intended for synchronous code within one component rather than between components—but it does seem like it could, since the synchronous nature would presumably be blocking. I'm just not sure search would get updated in the other component (I feel like I'd still have to get a fresh value from window)