Dev by Sofiia13 · Pull Request #172 · mate-academy/node_chat

Sofiia13 · 2026-02-04T13:20:29Z

Add solution

mateacademy-ai-mentor

Review Summary

Great work on building the overall structure for this chat application! The project is well-organized on both the frontend and backend. However, some changes are needed to fix a few critical issues that affect the core functionality.

Here are the main points to address:

Global Message Broadcasting: In backend/src/index.js, you're using io.emit(), which sends every message to every connected user, regardless of their room. To fix this, you'll need to have sockets join rooms (e.g., socket.join(roomId)) and then emit messages specifically to that room using io.to(roomId).emit(...).
Duplicate Messages for Sender: In frontend/src/pages/Chat/Chat.jsx, the user who sends a message sees it appear twice. This happens because you add the message to the local state immediately and then add it again when the server broadcasts it back. Please remove the local setMessagesList call in your handleSendMessage function and rely only on the socket.on('newMessage') listener to update the state.
Potential App Crash: The RoomsList component will crash if selectedRoom is null (like on first load) because you're trying to access selectedRoom.id. You should add a null check or use optional chaining (selectedRoom?.id) to prevent this error.

Once these key issues are resolved, the application will be working as expected. You're on the right track!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

backend/src/index.js

+  console.log('User connected:', socket.id);
+
+  socket.on('sendMessage', (data) => {
+    io.emit('newMessage', data);


The use of io.emit() here broadcasts the message to every connected client, which isn't suitable for a chat application with separate rooms. Messages should only be sent to clients who have joined the specific room where the message was sent. Consider implementing a mechanism for clients to join rooms (e.g., using socket.join(roomId)) and then broadcasting messages to that specific room using io.to(roomId).emit(...).

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

backend/src/routes/health.js

+const express = require("express");
+
+const router = express.Router();
+
+router.get("/health", (_req, res) => {
+  res.status(200).json({ status: "ok" });
+});
+
+module.exports = router;


This file uses the CommonJS module system (require and module.exports), while the rest of the backend project uses ES Modules (import and export). It's important to be consistent with the module system across a project to prevent errors. Please update this file to use ES Module syntax.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

backend/src/utils/fileStorage.js

+const getMessages = async (roomId) => {
+  const data = await readChat();
+  const room = data.rooms.find((roomItem) => roomItem.id === roomId);
+  return room ? room.messages : [];
+};


This getMessages function is well-implemented but it's currently not used anywhere in the project. The logic for retrieving messages is duplicated in the getMessages route handler in src/routes/messages.js. To improve code reuse, the route handler should call this utility function instead of re-implementing the logic.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

backend/src/utils/fileStorage.js

+const addMessage = async (roomId, message) => {
+  const data = await readChat();
+  const room = data.rooms.find((roomItem) => roomItem.id === roomId);
+
+  if (!room) {
+    return false;
+  }
+
+  room.messages.push(message);
+  await writeChat(data);
+  return true;
+};


Similar to the getMessages function above, this addMessage utility is also unused. The logic for adding a message to a room is re-implemented inside the addMessage route handler in src/routes/messages.js. You should refactor that handler to use this function to avoid code duplication.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

backend/src/routes/rooms.js

+
+const renameRoom = async (req, res) => {
+  const { roomId } = req.params;
+  const { newName } = req.body;


The newName variable is used without validation. If an empty string is sent in the request body, the room's name will be set to empty. You should add a check to ensure newName is a non-empty string, similar to the validation in the createRoom function.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

frontend/src/components/RoomsList/RoomsList.jsx

+        <div key={room.id}>
+          <Room
+            roomName={room.name}
+            isSelected={selectedRoom.id === room.id}


The selectedRoom prop could be null, for instance, when the application first loads or if there are no rooms. Accessing selectedRoom.id directly will cause a runtime error in that scenario. You should add a check to ensure selectedRoom exists before trying to access its id property. Optional chaining (selectedRoom?.id) would be a concise way to fix this.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

frontend/src/pages/Chat/Chat.jsx

+    };
+
+    loadMessages();
+  }, [selectedRoom, messagesList.length]);


The dependency array [selectedRoom, messagesList.length] causes this effect to re-run every time a new message is added to messagesList. This results in re-fetching all messages for the room on every new message, which is inefficient and unnecessary. This hook should likely only depend on selectedRoom to load messages when the user switches rooms.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

frontend/src/pages/Chat/Chat.jsx

+      socket.emit('sendMessage', response.data.message);
+      setMessagesList((prevMessages) => [
+        ...prevMessages,
+        response.data.message,
+      ]);


There are two issues here. First, the backend response for a new message is { message: 'Message added', data: newMessage }. You are emitting response.data.message, which is the string "Message added", not the message object. You should be using response.data.data.

Second, you are adding the message to the list here locally, and the socket.on('newMessage') listener will add it again when the server broadcasts it back. This will cause duplicate messages for the sender. A common pattern is to only rely on the websocket broadcast to update the state for all clients, including the sender. Consider removing the setMessagesList call from this function.

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

frontend/src/pages/Chat/Chat.jsx

+      await api.delete(`/rooms/${roomId}`);
+      setRooms((prevRooms) => prevRooms.filter((room) => room.id !== roomId));
+      if (selectedRoom && selectedRoom.id === roomId) {
+        setSelectedRoom(rooms[0] || null);


When deleting a room, you are using the rooms variable from the component's state to determine the next selected room. However, setRooms is asynchronous, and rooms here refers to the state before the update. To fix this, you should use the newly filtered array to determine the next selection.

For example:

const nextRooms = prevRooms.filter(room => room.id !== roomId); setRooms(nextRooms); if (selectedRoom && selectedRoom.id === roomId) { setSelectedRoom(nextRooms[0] || null); }

mateacademy-ai-mentor · 2026-02-04T13:24:02Z

frontend/src/pages/Chat/Chat.jsx

+        <Button onClick={handleSendMessage}>Send</Button>
+      </div>
+      {messagesList.map((msg, index) => (
+        <Message key={index} text={msg.text} sender={msg.author} />


Using the array index as a key is an anti-pattern in React, especially for lists that can change. It can lead to incorrect rendering and state management issues. It's better to use a unique and stable identifier from the data itself. Since your messages have a time property which should be unique, you could use that instead (e.g., key={msg.time}).

mateacademy-ai-mentor

Review Summary

Excellent progress on the chat application! You've successfully resolved the critical backend issues, and the socket communication is now correctly scoped to rooms. However, there are a few new critical issues on the frontend that need to be addressed before this can be approved.

Here are the main blockers:

Infinite API Calls: In frontend/src/pages/Chat/Chat.jsx, the useEffect hook for fetching messages includes messagesList.length in its dependency array. This creates an infinite loop of network requests every time a new message is received. Please remove it from the dependency array.
App Crash on Load: The RoomsList component at frontend/src/components/RoomsList/RoomsList.jsx will crash if selectedRoom is null (e.g., on first load). You can fix this by using optional chaining (selectedRoom?.id) to safely access the id.
Real-time Messages are Broken: When sending a message in Chat.jsx, you're emitting response.data.message via the socket, which is incorrect. The actual message object is in response.data.data. This prevents other users from seeing new messages.

You're very close to a fully functional app! Fixing these issues will make it stable and complete.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-02-04T13:45:54Z

backend/src/routes/messages.js

+    time: time || new Date().toISOString(),
+  };
+
+  room.messages.unshift(newMessage);


Using unshift here adds the newest message to the beginning of the array. However, on the frontend, new messages received via sockets are appended to the end of the messagesList state. This inconsistency will cause messages to appear out of order.

To fix this, you should use push here to add new messages to the end of the array. This will ensure that messages are always in chronological order, both when initially loaded and when received in real-time.

mateacademy-ai-mentor · 2026-02-04T13:45:54Z

backend/src/utils/fileStorage.js

+const getMessages = async (roomId) => {
+  const data = await readChat();
+  const room = data.rooms.find((roomItem) => roomItem.id === roomId);
+  return room ? room.messages : [];
+};
+
+const getRoomsUtil = async () => {
+  const data = await readChat();
+  return data.rooms;
+};
+
+const addMessage = async (roomId, message) => {
+  const data = await readChat();
+  const room = data.rooms.find((roomItem) => roomItem.id === roomId);
+
+  if (!room) {
+    return false;
+  }
+
+  room.messages.push(message);
+  await writeChat(data);
+  return true;
+};


It looks like the logic for fetching and adding messages is handled directly in your route handlers in src/routes/messages.js. As a result, these getMessages and addMessage utility functions are not being used. It's a good practice to remove unused code to keep the project clean and maintainable.

mateacademy-ai-mentor · 2026-02-04T13:45:54Z

frontend/src/components/RoomsList/RoomsList.jsx

+        <div key={room.id}>
+          <Room
+            roomName={room.name}
+            isSelected={selectedRoom.id === room.id}


This line will cause the application to crash if selectedRoom is null or undefined, which can happen when the application first loads or after a room is deleted. As mentioned in the previous review, you should use optional chaining (selectedRoom?.id) to safely access the id property. This will prevent the error.

mateacademy-ai-mentor · 2026-02-04T13:45:54Z

frontend/src/components/Room/Room.jsx

+      <button className={styles.room__button} onClick={onDelete}>
+        <img
+          className={styles.room__icon}
+          src="/delete.svg"
+          alt="Delete room"
+        />
+      </button>
+      <button className={styles.room__button} onClick={onEdit}>
+        <img className={styles.room__icon} src="/edit.svg" alt="Edit room" />
+      </button>


When a user clicks the delete or edit button, the onSelect event on the parent div also fires due to event bubbling. This causes the room to be selected right before it's deleted or edited, which is unintended behavior.

To fix this, you should modify the onClick handlers for these buttons to stop the event's propagation. For example:

onClick={(e) => { e.stopPropagation(); onDelete(); }}

mateacademy-ai-mentor · 2026-02-04T13:45:54Z

frontend/src/pages/Chat/Chat.jsx

+  useEffect(() => {
+    const loadMessages = async () => {
+      if (!selectedRoom?.id) return;
+
+      try {
+        const response = await api.get(`/api/${selectedRoom.id}/messages`);
+        const loadedMessages = response.data.messages || [];
+        setMessagesList(loadedMessages);
+      } catch (error) {
+        console.error(error);
+      }
+    };
+
+    loadMessages();
+  }, [selectedRoom, messagesList.length]);


Including messagesList.length in the dependency array for this useEffect will cause it to re-run every time a new message is received via the socket. This will trigger a new API call to fetch all messages again, creating an infinite loop of network requests.

This effect should only run when the user switches to a different room. Removing messagesList.length from the dependency array will fix this issue.

mateacademy-ai-mentor · 2026-02-04T13:45:54Z

frontend/src/pages/Chat/Chat.jsx

+      });
+      setMessage('');
+      socket.emit('sendMessage', {
+        ...response.data.message,


Based on your backend API in backend/src/routes/messages.js, the response object for adding a message is { message: 'Message added', data: newMessage }. This line is spreading response.data.message, which is just the string "Message added", not the message object.

To fix this, you should spread response.data.data instead to get the actual message object that includes the author, text, and time.

mateacademy-ai-mentor · 2026-02-04T13:45:55Z

frontend/src/pages/Chat/Chat.jsx

+      await api.delete(`/rooms/${roomId}`);
+      setRooms((prevRooms) => prevRooms.filter((room) => room.id !== roomId));
+      if (selectedRoom && selectedRoom.id === roomId) {
+        setSelectedRoom(rooms[0] || null);


The state update from setRooms is asynchronous, so when this line runs, the rooms variable still holds the old array of rooms (before the deletion). This can lead to selecting the wrong next room or trying to select the room that was just deleted.

To fix this, you should first create the new array of rooms, and then use that new array for both state updates:

const nextRooms = rooms.filter(room => room.id !== roomId); setRooms(nextRooms); if (selectedRoom && selectedRoom.id === roomId) { setSelectedRoom(nextRooms[0] || null); }

mateacademy-ai-mentor · 2026-02-04T13:45:55Z

frontend/src/pages/Chat/Chat.jsx

+        <Button onClick={handleSendMessage}>Send</Button>
+      </div>
+      {messagesList.map((msg, index) => (
+        <Message key={index} text={msg.text} sender={msg.author} />


Using an array's index as a key is not recommended, especially for lists that can be reordered or have items added/removed. It can lead to performance issues and bugs with component state.

Since each message has a unique timestamp, it would be better to use that as the key. For example: key={msg.time}.

mateacademy-ai-mentor

Review Summary

Excellent progress on your chat application! I am approving this submission as an educational override to help you continue moving forward, but I'd like you to review a few key points for future projects.

You've done a fantastic job fixing the previous issues and making the app stable. To make it even more robust, pay attention to a few things:

In frontend/src/pages/Chat/Chat.jsx, when deleting the selected room, be sure to use the new rooms list to set the next selected room to avoid using stale state.
In frontend/src/components/Room/Room.jsx, prevent clicks on the edit/delete buttons from also selecting the room by using e.stopPropagation() in their onClick handlers.
Also in Chat.jsx, remember to have the client leave the previous socket room when joining a new one to avoid receiving messages from multiple rooms.

There are also a couple of unused functions in the backend that you can clean up. Overall, this is a very strong submission, and you've built a complex, real-time application successfully. Keep up the great work!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

backend/src/index.js

+  socket.on('sendMessage', (data) => {
+    io.to(data.roomId).emit('newMessage', data);
+  });


This sendMessage event listener appears to be unused. The frontend sends messages via an HTTP POST request to /api/:roomId/messages, and the handler for that route is responsible for emitting the newMessage event. You can safely remove this block to clean up the code.

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

backend/src/utils/fileStorage.js

+const getMessages = async (roomId) => {
+  const data = await readChat();
+  const room = data.rooms.find((roomItem) => roomItem.id === roomId);
+  return room ? room.messages : [];
+};


The getMessages function is defined and exported but is not used anywhere in the application. The logic to get messages is handled directly within the /api/:roomId/messages route handler. You can remove this function to clean up the code.

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

backend/src/utils/fileStorage.js

+const addMessage = async (roomId, message) => {
+  const data = await readChat();
+  const room = data.rooms.find((roomItem) => roomItem.id === roomId);
+
+  if (!room) {
+    return false;
+  }
+
+  room.messages.push(message);
+  await writeChat(data);
+  return true;
+};


Similar to getMessages, the addMessage function is also defined and exported but remains unused. The message creation logic is handled inside the POST route handler in messages.js. This function can be safely removed.

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

frontend/src/components/Room/Room.jsx

+      <button className={styles.room__button} onClick={onDelete}>
+        <img
+          className={styles.room__icon}
+          src="/delete.svg"
+          alt="Delete room"
+        />
+      </button>
+      <button className={styles.room__button} onClick={onEdit}>
+        <img className={styles.room__icon} src="/edit.svg" alt="Edit room" />
+      </button>


When the delete or edit buttons are clicked, the onClick event will bubble up to the parent div, triggering the onSelect handler. This can cause unexpected behavior, such as selecting a room just before it's deleted or edited. You should stop the event propagation in the onClick handlers for these buttons.

For example:

<button className={styles.room__button} onClick={(e) => { e.stopPropagation(); onDelete(); }} >

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

frontend/src/pages/Chat/Chat.jsx

+  useEffect(() => {
+    if (!selectedRoom?.id) {
+      return;
+    }
+
+    setMessagesList([]);
+
+    const joinRoom = () => {
+      socket.emit('joinRoom', selectedRoom.id);
+    };
+
+    joinRoom();
+    socket.on('connect', joinRoom);
+
+    return () => {
+      socket.off('connect', joinRoom);
+    };
+  }, [selectedRoom]);


When a user switches rooms, the client joins the new room but never leaves the previous one. This means the client remains subscribed to multiple rooms and will receive messages from all of them, displaying them incorrectly in the currently active room. You should implement logic to leave the previous room when a new one is selected.

A common way to handle this is to emit a leaveRoom event in the cleanup function of this useEffect hook. You would also need to add a corresponding leaveRoom event handler on your backend socket server.

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

frontend/src/pages/Chat/Chat.jsx

+      await api.delete(`/rooms/${roomId}`);
+      setRooms((prevRooms) => prevRooms.filter((room) => room.id !== roomId));
+      if (selectedRoom && selectedRoom.id === roomId) {
+        setSelectedRoom(rooms[0] || null);


There's a bug here when deleting the currently selected room. The line setSelectedRoom(rooms[0] || null) uses the rooms state variable, which is stale at this point because React state updates are asynchronous. This can lead to incorrect behavior, such as not selecting a new room or selecting the wrong one.

To fix this, you should first calculate the new array of rooms after filtering, and then use that new array to update both the rooms state and the selectedRoom state.

mateacademy-ai-mentor · 2026-02-04T13:58:03Z

frontend/src/pages/Chat/Chat.jsx

+        <Button onClick={handleSendMessage}>Send</Button>
+      </div>
+      {messagesList.map((msg, index) => (
+        <Message key={index} text={msg.text} sender={msg.author} />


Using the array index as a key is an anti-pattern in React, especially for lists where items can be added or removed. This can lead to incorrect rendering and state management issues. Each message has a unique time property, which would be a much better candidate for a stable key.

Consider changing this to key={msg.time}.

Sofiia13 added 5 commits February 3, 2026 16:51

create react and express app

f2b6282

add basic components to the chat

5ce6086

write api for rooms and messages

fe0a062

add frontend and connect webSockets

efb53be

add editing and deleting

5f43595

mateacademy-ai-mentor suggested changes Feb 4, 2026

View reviewed changes

fix errors

6bd76c0

Sofiia13 requested a review from mateacademy-ai-mentor February 4, 2026 13:28

delete file

ce2662a

mateacademy-ai-mentor suggested changes Feb 4, 2026

View reviewed changes

fix

71a22f5

Sofiia13 requested a review from mateacademy-ai-mentor February 4, 2026 13:54

mateacademy-ai-mentor approved these changes Feb 4, 2026

View reviewed changes

Conversation

Sofiia13 commented Feb 4, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants