Question 52894 in Javascript Question posted in
A very good W3school tutorial can be found here.

Executing extra code inside setState callback – Javascript

I am executing extra code to send a webSocket message inside the setState callback, and I want to know if what I am doing is correct or can lead to some problems. this is what I have:

const setLocalMessageData = (body: string, sendMessage: SendMessage) => {
  setMessagesData((messages) => {
    const number = messages.length
    sendMessage(number, body)
    const message = {user: LOCAL_USER, number: number, body: body, ack: false}
    return [...messages, message]
  })
}

setMessagesData is the setState and sendMessage is the extra code that transmits a webSocket message.

I expect that te setLocalMessageData transmits the webSocket message and update messagesData properly.

Tags:

2

Answers


  1. 0

    you should rewrite it in this way:

    useEffect(() => {
  const number = messages.length;

  sendMessage(number, body);
}, [messages]);
    Login or Signup to reply.
  2. 0

    I disagree with the other answers posted here; a useEffect() should not be introduced here. This is due to a couple of reasons which are covered thoroughly in the new React docs.

    The first is that in React StrictMode, effects are run twice, meaning during development, an effect would send two messages instead of just one. This can lead to unpredictable and unintended behavior during development. While this can be fixed with the appropriate cleanup function, there is another issue.

    The other reason is that effects are a tool for synchronizing your component with an external system. However, the user isn’t external; if you want something to happen because the user did something, then that logic should be in an event handler, not useEffect(). Event handlers are guaranteed to only execute in response to some user interaction. You can read more about situations in which you don’t need an effect here.

    Moreover, React batches state updates. So if something were to update your messages list with multiple new messages, useEffect would only send the most recent body through your WebSocket, not all new messages.

    And finally, if you are planning to implement receiving messages from the WebSocket, then updating the array with the received message would cause the useEffect to send the last message it received back through WebSocket, potentially creating an infinite message loop between clients.

    Therefore, I recommend you simply define a new function within your component to be your event handler, and send the message within it like so:

    // use this instead of wherever `setLocalMessageData()` was being called.
function submitMessage() {
  const messageCount = messages.length;
  const newMessage = {user: LOCAL_USER, number: number, body, ack: false};
  setMessagesData(messages => [...messages, newMessage]);
  
  sendMessage(messageCount, body);
}

    Also, to clarify the issue with your initial approach: React expects set functions to be pure (have no side effects), so it also calls them twice during development when in StrictMode, which would cause your message to get sent twice as well.

    EDIT: If this component is handling receiving messages from the WebSocket as well, then there potentially may be a race condition where a new message might come in at the same time (in the same event loop iteration) that the user is sending one themselves. However, if the received messages are handled first, then messages.length in the other handler will not refer to the correct number of messages because React has not had the chance to rerender yet.

    In this situation, I think it’s best to declare a Ref in your component for synchronously accessing and updating the number of total messages (thanks @Emile Bergeron). It should look roughly something like the following:

    function SomeComponent() {
  const [messages, setMessagesData] = useState([]);
  const syncCount = useRef(messages.length);
  syncCount.current = messages.length; // sync with true state
  // ...

  useEffect(() => {
    const handleOnMessage = (evt) => {
      const newMessages = getNewMessages(evt.data);
      setMessagesData((messages) => [...messages, ...newMessages]);
      syncCount.current += newMessages.length;
    };

    socket.addEventListener("message", handleOnMessage);

    return () => {
      socket.removeEventListener("message", handleOnMessage);
    };
  }, []);
  // ...

  function submitMessage() {
    const newMessage = {user: LOCAL_USER, number: number, body, ack: false};
    setMessagesData(messages => [...messages, newMessage]);
    sendMessage(syncCount.current, body);
    syncCount.current += 1;
  }
  // ...
}

    For reference, here is a CodeSandbox which recreates the race condition, and implements the described fix.

    However, in my honest opinion, it seems a bit strange to me that you are sending the message’s index over the WebSocket; I feel it’s unnecessary. The server which manages the WebSockets should be the source of truth for the message data and its order. In other words, clients should not be asserting what they think the order of messages is to the server, but rather the server should be communicating to clients the order of messages (most likely based on the timestamp the messages were received by the server). That way, your React code doesn’t need to worry about race conditions anymore because any uncertainty or inconsistency is resolved by the server.

    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search