Avoiding race conditions with react hooks and native callbacks

Question

I'm trying to write some inter-frame-comunication hook and I'm not sure that the implementation is correct. Unfortunately, the react lifecycle topic seems very complex (example) and I couldn't find a definite answer or recommendation about how to implement it correctly.

Here's my attempt at writing the hook:

const frame = /*...*/;
let messageId = 0;

function usePostMessage(
  eventName: string,
  handler: (success: boolean) => void
) {
  const [pendingMessageId, setPendingMessageId] = useState<number>();

  const postMessage = useCallback(() => {
    frame.postMessage(eventName);
    setPendingMessageId(++messageId);
  }, [eventName]);

  useEvent(
    "message",
    useCallback(
      (message) => {
        if (
          message.eventName === eventName &&
          message.messageId === pendingMessageId
        ) {
          handler(message.success);
          setPendingMessageId(undefined);
        }
      },
      [eventName, handler, pendingMessageId]
    )
  );

  return { postMessage, pendingMessageId };
}

(I'm using useEvent)

Usage:

const { postMessage, pendingMessageId } = usePostMessage(
  "eventName",
  (success) => {
    console.log("eventName", success ? "succeeded" : "failed");
  }
);

if (pendingMessageId !== undefined) {
  return <div>Pending...</div>;
}

return <button onclick={postMessage}>Click me</button>;

As you can see, I tried to implement a way to post a message and get a response from a frame. I also tried to avoid pitfalls such as getting unrelated responses by keeping a message counter.

It works, but I'm afraid that the "message" event might arrive before the setPendingMessageId state is updated. Is that possible? Are there any guidelines or best practices for implementing this correctly? Thanks.

score 0 · Answer 1 · answered Dec 17 '22 at 15:32

0

Update the setPendingMessageId inside the useEffect hook

useEffect(() => {
    setPendingMessageId(++messageId);
  }, [postMessage])

state update is applied after the postMessage function has been called, avoiding the race condition.

answered Dec 17 '22 at 15:32

Sachin

149
1
16

score 0 · Answer 2 · answered Dec 17 '22 at 15:43

0

I'm afraid that the "message" event might arrive before the setPendingMessageId state is updated. Is that possible?

No. If a state setter is called inside a React function (such as an onclick prop, as in your code), React will re-render a component after that React handler finishes running its code. JavaScript is single-threaded; once setPendingMessageId(++messageId); is called, the click handler will end, and then a re-render will occur. There's no chance of any other code running before then. The receipt of the message goes through a non-React API (the message listener on the window), so React doesn't try to integrate it into the rerendering flow.

That said, although your code will work, to avoid having to worry about this, some might prefer to reference the stateful values as they are when the message is posted rather than put the logic in a separate hook, which could be less reliable if the state gets out of sync for some other reason. So instead of useEvent, you could consider something along the lines of

const postMessage = useCallback(() => {
  frame.postMessage(eventName);
  setPendingMessageId(++messageId);
  // Save a reference to the current value
  // just in case it changes before the response
  const thisMessageId = messageId;
  const handler = ({ data }) => {
    if (data.eventName === eventName && data.messageId === thisMessageId) {
      handler(data);
      window.removeEventListener('message', handler);
    }
  };
  window.addEventListener('message', handler);
}, [eventName]);

Having a messageId outside of React is a little bit smelly. It'd be nice if you could integrate it into state somehow (perhaps in an ancestor component) and then add it to the dependency array for postMessage.

answered Dec 17 '22 at 15:43

CertainPerformance

356,069
52
309
320

Thanks, "If a state setter is called inside a React function..." - yes, but the handler is updated with `useEffect`, [see `useEvent` implementation](https://github.com/streamich/react-use/blob/master/src/useEvent.ts#L41) ; "to avoid having to worry about this..." - this looks more convincing to be race free, but if a reply never arrives, the handler keeps hanging, and if it does after the element is gone, weird stuff will happen. – Paul Dec 17 '22 at 15:51
The issue is that the state update is depending on a *non* React handler - the message from the other window to the current window. `useEvent` is only providing an interface for the native DOM approach - the (non-React) window fires the event, and React doesn't have a way to see if any such events are being fired when preparing to update without possibly running lots more code outside of React, so it doesn't wait for it before re-rendering. See [here](https://stackoverflow.com/a/70889146). – CertainPerformance Dec 17 '22 at 15:56
Sorry, I'm not sure I follow. The link you posted doesn't mention `useEffect`. Are you still saying that my implementation is safe? Because what I see is the following that can happen: 1. `postMessage()`, 2. `setPendingMessageId(++messageId)`, 3. `addEventListener` called with the old callback, 4. `useEvent` updates the callback via `useEffect`. ... I'm just looking for the most robust solution, hoping that React has a well known and recommended way to tackle it. – Paul Dec 17 '22 at 16:10
Your implementation is safe because *3. `addEventListener` called with the old callback* will not occur - even if the response came immediately, because the response is triggered outside of a React function (via `.addEventListener`), for that to run, React will have to have yielded control back to the browser engine so that the browser can process the message and fire the message event. But it'll only yield control back to the engine once it's finished re-rendering. – CertainPerformance Dec 17 '22 at 16:17
OK, that's, that's good to know but that's the kind of information I couldn't find online, and [the link I posted](https://stackoverflow.com/q/68407187/2604492) says otherwise. Can you please post a reference for it, maybe from the React documentation? – Paul Dec 17 '22 at 16:20

Avoiding race conditions with react hooks and native callbacks

2 Answers2