React/Socket.io not displaying latest message passed down as prop

Question

I am working on a chat application using React and socket.io. Back end is express/node. The relevant components are: Room.js --> Chat.js --> Messages.js --> Message.js

messageData received from the server is stored in state in Room.js. It is then passed down through Chat.js to Messages.js, where it is mapped onto a series of Message.js components.

When messages are received, they ARE appearing, but only after I start typing in the form again, triggering messageChangeHandler(). Any ideas why the Messages won't re-render when a new message is received and added to state in Room.js? I have confirmed that the state and props are updating everywhere they should be--they just aren't appearing/re-rendering until messageChangeHandler() triggers its own re-render.

Here are the components.

Room.js

export default function Room(props) {
    const [messagesData, setMessagesData] = useState([])

    useEffect(() => {
        console.log('the use effect ')
        socket.on('broadcast', data => {
            console.log(messagesData)
            let previousData = messagesData
            previousData.push(data)
            // buildMessages(previousData)
            setMessagesData(previousData)
        })
    }, [socket])


    console.log('this is messagesData in queue.js', messagesData)

    return(
        // queue counter will go up here
        // <QueueDisplay />

        // chat goes here
        <Chat 
            profile={props.profile} 
            messagesData={messagesData}
        />

    )
}

Chat.js

export default function Chat(props) {
    // state
    const [newPayload, setNewPayload] = useState({
        message: '',
        sender: props.profile.name
    })
    // const [messagesData, setMessagesData] = useState([])
    const [updateToggle, setUpdateToggle] = useState(true)


    const messageChangeHandler = (e) => {
        setNewPayload({... newPayload, [e.target.name]: e.target.value})
    }

    const messageSend = (e) => {
        e.preventDefault()
        if (newPayload.message) {
            socket.emit('chat message', newPayload)
            setNewPayload({
                message: '',
                sender: props.profile.name  
            })
        }
    }
    
    return(
        <div id='chatbox'>
            <div id='messages'>
                <Messages messagesData={props.messagesData} />
            </div>
            <form onSubmit={messageSend}>
                <input 
                    type="text" 
                    name="message" 
                    id="message" 
                    placeholder="Start a new message" 
                    onChange={messageChangeHandler} 
                    value={newPayload.message}
                    autoComplete='off'
                />
                <input type="submit" value="Send" />
            </form>
        </div>
    )
}

Messages.js

export default function Messages(props) {
    return(
        <>
        {props.messagesData.map((data, i) => {
            return <Message key={i} sender={data.sender} message={data.message} />
        })}
        </>
    )
}

Message.js

export default function Message(props) {
    return(
        <div key={props.key}>
            <p>{props.sender}</p>
            <p>{props.message}</p>
        </div>
    )
}

Thank you in advance for any help!

Unprepared

Have you tried to set the state in Room component like setMessagesData([...messageData, data])? – Alage

@Alage That's what I had originally, and I just tried it again. For some reason, that overwrites the state entirely, meaning the state will only hold the most recent message, so it won't work. Thanks for checking. – Unprepared

That's because useEffect() doesn't have a dependency on messagesData, so messagesData was changing outside useEffect() but not inside it. More details in my overly-elaborate answer. – Jairia

Answer 1

I don't think that your useEffect() function does what you think it does.

Red flag

Your brain should generate an immediate red flag if you see a useEffect() function that uses variables declared in the enclosing scope (in a closure), but those variables are not listed in useEffect()'s dependencies (the [] at the end of the useEffect())

What's actually happening

In this case, messagesData in being used inside useEffect() but not declared as a dependency. What happens is that after the first broadcast is received and setMessagesData is called, messagesData is no longer valid inside useEffect(). It refers to an array, from the closure when it was last run, which isn't assigned to messageData any longer. When you call setMessagesData, React knows that the value has been updated, and re-renders. It runs the useState() line and gets a new messagesData. useEffect(), which is a memoized function, does NOT get recreated, so it's still using messagesData from a previous run.

How to fix it

Clean up useEffect()

Before we start, let's eliminate some of the noise in the function:

    useEffect(() => {
        socket.on('broadcast', data => {
            setMessagesData([...messagesData, data])
        })
    }, [socket])

This is functionally equivalent to your code, minus the console.log() messages and the extra variable.

Let's go one step further and turn the handler into a one-liner:

    useEffect(() => {
        socket.on('broadcast', data => setMessagesData([...messagesData, data]));
    }, [socket])

Add missing dependencies

Now, let's add the missing dependencies!

    useEffect(() => {
        socket.on('broadcast', data => setMessagesData([...messagesData, data]));
    }, [socket, messagesData])

Technically, we also depend on setMessagesData(), but React has this to say about setState() functions:

React guarantees that setState function identity is stable and won’t change on re-renders. This is why it’s safe to omit from the useEffect or useCallback dependency list.

Too many cooks

The useEffect() function is looking good, but we still depend on messagesData. This is a problem, because every time socket receives a broadcast, messagesData changes, so useEffect() is re-run. Every time it is re-run, it adds a new handler/listener for broadcast messages, which means that when the next message is received, every handler/listener calls setMessagesData(). The code might still accidentally work, at least logic-wise, because listeners are usually called, synchronously, in the order in which they were registered, and I believe that if multiple setState() calls are made during the same render, React only re-renders once using the final setState() call. But it will definitely be a memory leak, since we have no way to unregister all of those listeners.

This tiny problem would normally end up being a huge pain to solve, because to fix this problem, we would need to unregister the old listener every time we registered a new one. And to unregister a listener, we call removeListener() function with the same function we registered - but we don't have that function anymore. Which means we need to save the old function as state or memoize it, but now we also have another dependency for our useEffect() function. Avoiding a continuous loop of infinite re-renders turns out to be non-trivial.

The trick

It turns out that we don't have to jump through all of those hoops. If we look closely at our useEffect() function, we can see that we don't actually use messagesData, except to set the new value. We're taking the old value and appending to it.

The React devs knew that this was a common scenario, so there's actually a built-in helper for this. setState() can accept a function, which will immediately be called with the previous value as an argument. The result of this function will be the new state. It sounds more complicated than it is, but it looks like this:

setState(previous => previous + 1);

or in our specific case:

setMessagesData(oldMessagesData => [...oldMessagesData, data]);

Now we no longer have a dependency on messagesData:

    useEffect(() => {
        socket.on('broadcast', data => setMessagesData(oldMessagesData => [...oldMessagesData, data]);
    }, [socket])

Being polite

Remember earlier when we talked about memory leaks? It turns out this can still happen with our latest code. This Component may get mounted and unmounted multiple times (for example, in a Single-Page App when the user switches pages). Each time this happens, a new listener is registered. The polite thing to do is to have useEffect() return a functions which will clean up. In our case this means unregistering/removing the listener.

First, save the listener before registering it, then return a function to remove it

    useEffect(() => {
        const listener = data => setMessagesData(oldMessagesData => [...oldMessagesData, data];
        socket.on('broadcast', listener);
        return () => socket.removeListener('broadcast', listener);
    }, [socket])

Note that our listener will still be dangling if socket changes, and since it's not clear in the code where socket comes from, whatever changes that will also have to remove all old listeners, e.g. socket.removeAllListeners() or socket.removeAllListeners('broadcast').

Jairia

Answer 2

Changing the useEffect in room to contain the following fixed the issue:

    useEffect(() => {
        console.log('the use effect ')
        socket.on('broadcast', data => {
            console.log(messagesData)
            // let previousData = messagesData
            // previousData.push(data)
            // setMessagesData(previousData)
            setMessagesData(prev => prev.concat([data]))
        })
    }, [socket])```

Unprepared