solution by nastia7408 · Pull Request #205 · mate-academy/node_chat

nastia7408 · 2026-04-30T14:11:50Z

No description provided.

mateacademy-ai-mentor

Review Summary

This chat application has the core functionality implemented correctly - room operations (create/rename/join/delete), message display with author/time/text, localStorage persistence, and real-time messaging all work. However, there are critical issues that must be fixed before approval:

Critical Issues:

React state mutation during render (ChatInfoModal.jsx lines 40-43, ChatInfoModal.scss, AuthPage.scss, CreateChatModal.jsx) - Calling setPrevChatId directly in the component body instead of in useEffect causes infinite re-renders and breaks the application. Move all conditional state updates into useEffect hooks with proper dependencies.
Hardcoded database credentials (server/src/db.js) - Credentials are directly in source code. Use environment variables (process.env.DB_USER, process.env.DB_PASSWORD) for security.
Socket.IO broadcasts to all clients (server/src/server.js line 261) - Using io.emit() notifies every connected user. Use io.to(chatId).emit() to only notify members of the relevant chat.

Minor Improvements:

CORS configuration (server/src/server.js) only allows GET/POST but the app uses PUT and DELETE methods
Accessibility: Use <button> elements instead of <div onClick> in AuthPage.jsx and MainPage.jsx
Empty chat name validation is missing in CreateChatModal

These issues affect application stability and security, so they need to be addressed before this can be approved.

✨ 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-04-30T14:15:33Z

+import { StrictMode } from "react";
+import { createRoot } from "react-dom/client";
+
+import App from "./App.jsx";
+
+createRoot(document.getElementById("root")).render(
+  <StrictMode>
+    <App />
+  </StrictMode>
+);


The task requires 'You type a username and send it to the server', but this implementation requires full registration (name, phone, email, password). The requirement appears to be simpler - just a username. Consider simplifying to just accept a username without requiring email/phone/password.

mateacademy-ai-mentor · 2026-04-30T14:15:33Z

+    border-radius: 50%;
+  }
+  .add-chat {
+    font-weight: bold;
+    font-size: 35px;
+    padding-bottom: 6px;
+  }
+  .logout-button {
+    background-color: #222329;
+    border: none;
+    color: #6b6375;
+    padding-top: 5px;
+  }
+}
+
+.chats-block {
+  width: 35%;
+  padding-top: 10px;
+  border-right: 1px solid #222329;
+  overflow-y: auto;
+
+  &::-webkit-scrollbar,
+  .chats-block::-webkit-scrollbar {
+    width: 6px;
+  }
+
+  &::-webkit-scrollbar-track,
+  .chats-block::-webkit-scrollbar-track {
+    background: transparent;
+  }
+
+  &::-webkit-scrollbar-thumb,
+  .chats-block::-webkit-scrollbar-thumb {
+    background-color: #36313b;
+    border-radius: 10px;
+  }
+
+  &::-webkit-scrollbar-thumb:hover,
+  .chats-block::-webkit-scrollbar-thumb:hover {
+    background-color: #6b6375;


The task requires 'typing a username' for authentication, but this implementation requires full registration with name, phone, email, and password. Consider whether a simpler username-only flow aligns better with the task description.

mateacademy-ai-mentor · 2026-04-30T14:15:33Z

+            required
+          />
+          <button className="button" type="submit">{isRegister ? "Register" : "Login"}</button>
+        </form>


Use a <button> element instead of <p> with onClick for the register/login toggle. This improves accessibility and keyboard navigation support.

mateacademy-ai-mentor · 2026-04-30T14:15:33Z

+  const [prevChatId, setPrevChatId] = useState(chat.id);
+  if (chat.id !== prevChatId) {
+    setPrevChatId(chat.id);
+    setNewName(chat.name || "");


The if statement on line 40 checking chat.id !== prevChatId is a React anti-pattern - it runs during render without being in useEffect or useLayoutEffect. This can cause rendering issues. Move this logic inside a useEffect hook.

mateacademy-ai-mentor · 2026-04-30T14:15:33Z

+    setIsLoading(true);
+    try {


Consider checking response.ok before calling response.json() to properly handle server error responses. If the server returns an error status, response.json() may throw or return unexpected data.

mateacademy-ai-mentor · 2026-04-30T14:15:34Z

+    return () => {
+      socket.off("receive_message");
+      socket.off("update_chat_list");
+    };


The socket.on callbacks depend on selectedChat, but MY_ID is used inside without being in the dependency array. If MY_ID changes (unlikely given the flow but theoretically possible), this could cause issues. Consider adding MY_ID to the dependency array.

mateacademy-ai-mentor · 2026-04-30T14:15:34Z

+                  className="send-message-field"
+                  placeholder="Write a message..."
+                  value={messageText}
+                  onChange={(e) => setMessageText(e.target.value)}


The pattern e.key === 'Enter' && sendMessage() works but is unconventional. Consider using a block: if (e.key === 'Enter') sendMessage() for better readability and consistency.

mateacademy-ai-mentor · 2026-04-30T14:15:34Z

+
+- You type a username and send it to the server
+- It is now username (save it in localStorage)
+- All the messages should have an author, time and text
+- Implement an ability to create rooms (create / rename / join / delete)
+- New user should see all prev messages in the room


Database credentials are hardcoded directly in the code. Move these to environment variables for security: process.env.DB_USER, process.env.DB_PASSWORD, etc.

mateacademy-ai-mentor · 2026-04-30T14:15:34Z

+
+    const messageData = newMessage.rows[0];
+
+    io.to(chatId.toString()).emit('receive_message', messageData);


Using io.emit('update_chat_list', messageData) sends updates to ALL connected clients. Consider broadcasting only to users who are members of the chat, or use rooms: io.to(chatId).emit(...)

mateacademy-ai-mentor · 2026-04-30T14:15:34Z

+  }
+});
+
+app.delete('/api/chats/:id', async (req, res) => {


When deleting a chat, associated messages remain in the database. Consider deleting messages first: DELETE FROM messages WHERE chat_id = $1 before deleting the chat, to prevent orphaned data.

mateacademy-ai-mentor

Review Summary

Your chat application implementation shows good progress with proper Socket.IO integration, routing setup, and message handling. However, there are several critical issues to address: AuthPage.jsx sends formData.username which is undefined - the formData state has name, phone, email, password fields, so adjust your payload accordingly (for registration use all fields, for login use only email and password). ChatInfoModal.jsx lines 47-51 call setPrevChatId and setNewName directly during render causing infinite re-renders - remove this redundant code since lines 17-22 already handle this correctly in a useEffect. server.js line 262 uses io.emit() broadcasting to all users - change to io.to(chatId.toString()).emit() so only chat members receive updates. UserInfoModal.jsx should add a null check for the user prop before accessing its properties. For future improvements, consider replacing <div> clickable elements with semantic <button> elements for better accessibility. These are common React patterns to master, and fixing them will make your application production-ready!

✨ 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-04-30T17:45:18Z

+    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
+    <title>my-node_chat-front</title>
+  </head>
+  <body>


Duplicate entry: node_modules is listed twice (lines 6 and 9). Remove one of these entries to keep the file clean.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+    <BrowserRouter>
+      <Routes>
+        <Route
+          path="/auth"


Critical bug: formData.username is always undefined because the formData state has name, phone, email, password - no username field. For registration, the payload should include the actual user fields (name, phone, email, password).

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+          path="/auth"
+          element={!user ? <AuthPage onLoginSuccess={setUser} /> : <Navigate to="/" />}
+        />
+
+        <Route
+          path="/"
+          element={


The same payload { username: formData.username } is sent for both register and login, but the server expects { name, phone, email, password } for register and { email, password } for login.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+  const handleSubmit = async (e) => {
+    e.preventDefault();
+    const endpoint = isRegister ? '/api/register' : '/api/login';
+


Critical bug: formData.username is undefined because the state only has name, phone, email, password. For registration, the payload should include the fields the server expects: name, phone, email, password. For login, it should include email and password. Currently, neither registration nor login will work correctly.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+}
+
+.main {
+  display: flex;


Critical bug: formData.username doesn't exist in the state. The state has name, phone, email, password fields (lines 6-10), but line 16 tries to access username. This causes the authentication request to always send undefined as the username. Either add a username field to the formData state and form inputs, or change line 16 to use an existing field like formData.name.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+  border-radius: 5px;
+  padding: 5px;
+  color: #6b6375;
+  font-weight: 600;
+  font-size: 14px;


Critical bug - React state mutation during render: setPrevChatId and setNewName are called directly in the component body instead of inside useEffect. This causes infinite re-renders when chat.id changes. This logic already exists correctly in the useEffect at lines 17-22. Either remove lines 47-51 or move them into a useEffect hook.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+
+  const [prevChatId, setPrevChatId] = useState(chat.id);
+  if (chat.id !== prevChatId) {
+    setPrevChatId(chat.id);
+    setNewName(chat.name || '');


Critical bug - React state mutation during render: Lines 48-51 call setPrevChatId and setNewName directly in the component body instead of in a useEffect. This causes infinite re-renders. Either remove lines 47-51 (since lines 17-22 already handle this correctly with useEffect), or move this logic into a useEffect with proper dependencies.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+    if (chat.id !== prevvChatId) {
+      setPrevvChatId(chat.id);
+      setNewName(chat.name || '');
+    }


Unnecessary dependency: setNewName is a stable reference from useState and doesn't need to be in the dependency array. Remove it from the useEffect on line 22.

mateacademy-ai-mentor · 2026-04-30T17:45:18Z

+        <div className="modal-body">
+          <input
+            type="text"
+            placeholder="Enter chat name..."


Critical bug - React state mutation during render: Lines 48-51 set state directly during the render phase (setPrevChatId, setNewName). This causes infinite re-renders because each state update triggers another render. Move this logic into a useEffect hook with proper dependencies.

Additionally, this component has TWO mechanisms tracking the previous chat ID:

prevvChatId (line 15) with a useEffect (lines 17-22)

prevChatId (line 47) with direct state mutation (lines 48-51)

This is redundant. Remove one mechanism entirely and keep the other properly inside useEffect.

mateacademy-ai-mentor · 2026-04-30T17:45:23Z

+
+      const response = await fetch("http://localhost:5000/api/chats", {
+        method: "POST",
+        headers: { "Content-Type": "application/json" },


The setNewName dependency in the useEffect is a setter function from useState. Setter functions are stable references and don't need to be in dependency arrays. This may cause unnecessary re-runs of the effect. Remove setNewName from the dependency array.

mateacademy-ai-mentor · 2026-04-30T17:45:23Z

+  display: flex;
+  align-items: center;
+}
+
+.input-block {


Critical bug - infinite re-render: Lines 48-51 call setPrevChatId and setNewName directly in the component body during render. This causes infinite re-renders because the state update triggers a re-render, which sees the same prevChatId value and calls setPrevChatId again. Move this logic into a useEffect hook, or better yet, remove lines 47-51 entirely since lines 17-22 already handle this correctly in a useEffect.

mateacademy-ai-mentor · 2026-04-30T17:45:23Z

+  font-weight: 400;
+  color: #6b6375;
+  background-color: #16171d;
+  padding: 10px 30px;
+  border-radius: 15px;
+  width: 500px;
+  height: auto;
+  box-shadow: 0 10px 25px rgba(0, 0, 0, 0.5);


Redundant state: prevvChatId (line 15) and prevChatId (line 47) are both tracking chat.id for the same purpose. This creates confusion and maintenance issues. Keep only one approach - the useEffect at lines 17-22 is the correct one.

mateacademy-ai-mentor · 2026-04-30T17:45:23Z

+  border-radius: 15px;
+  width: 500px;
+  height: auto;
+  box-shadow: 0 10px 25px rgba(0, 0, 0, 0.5);


Suspicious dependency: setNewName is included in the useEffect dependency array (line 22), but it's a state setter function that should never change. This is likely a mistake. Remove it from the dependencies.

mateacademy-ai-mentor · 2026-04-30T17:45:23Z

+      width: 100%;
+      height: 45px;
+      background-color: #16171d;
+      border: 2px solid #36313b;
+      border-radius: 10px;
+      padding: 0 15px;
+      color: #726b7b;
+      outline: none;
+
+      &:focus {


Critical bug - Memory leak and incorrect chat switching: The socket.on('receive_message') listener is added inside the useEffect (lines 57-64), but it's never removed when selectedChat changes. When a user switches chats, the old listener remains attached while a new one is added. This causes:

Memory leaks from accumulated listeners

Messages from old chats appearing in new chats

Messages being duplicated

Solution: Either move the listener setup to a separate useEffect with selectedChat as a dependency that cleans up properly, or ensure the cleanup function properly handles the conditional listener.

mateacademy-ai-mentor · 2026-04-30T17:45:24Z

+
+    .modal-input {
+      width: 100%;
+      height: 45px;


The join_chat event is only emitted on initial mount (line 56). When selectedChat changes, the user doesn't join the new chat room. Move the socket.emit('join_chat', selectedChat.id) call to a separate useEffect with selectedChat as a dependency, with proper cleanup to leave the previous chat room.

mateacademy-ai-mentor · 2026-04-30T17:45:24Z

+      width: 100%;
+      height: 45px;
+      background-color: #16171d;
+      border: 2px solid #36313b;
+      border-radius: 10px;
+      padding: 0 15px;
+      color: #726b7b;
+      outline: none;
+
+      &:focus {
+        background-color: #2d2835;
+      }
+    }
+
+    .create-btn {
+      background-color: #16171d;
+      border: 3px solid #4c4158;


Minor: The conditional logic on lines 55-65 could be extracted into a separate useEffect to make the dependency array cleaner and avoid the listener accumulation issue.

mateacademy-ai-mentor · 2026-04-30T17:45:24Z

+.user-modal-content {
+  font-weight: 400;
+  color: #6b6375;


Critical null safety issue: user prop is accessed directly without null checking. If user is null or undefined, user.name (line 14), user.phone (line 15), and user.email (line 16) will throw a runtime error. Add a guard like {user ? ... : <div>No user data</div>} or check if (!user) return null; at the start of the component.

mateacademy-ai-mentor · 2026-04-30T17:45:24Z

+      <header className="header">
+        <div className="title" onClick={() => setSelectedChat(null)}>
+          Chats


Accessibility issue: <div> used for a clickable element. Use a <button> element for clickable header actions instead of <div>.

mateacademy-ai-mentor · 2026-04-30T17:45:24Z

+        <div className="header-left-data">
+          <div
+            className="add-chat"
+            onClick={() => setIsCreateChatModalOpen(true)}
+          >
+            +


Accessibility issue: <div> used for the add chat button. Use a <button> element instead of <div> with onClick.

mateacademy-ai-mentor · 2026-04-30T17:45:28Z

+          </div>
+          <img
+            onClick={() => setIsUserInfoModalOpen(true)}
+            className="user-photo"
+            src={userPhoto}
+            alt="user_photo"


Accessibility issue: <img> should be wrapped in a <button> if it's clickable, not directly handling onClick.

mateacademy-ai-mentor · 2026-04-30T17:45:28Z

+              <div className="chat-header">
+                <img
+                  onClick={() => setIsChatInfoModalOpen(true)}
+                  className="chat-photo"
+                  src={userPhoto}
+                  alt="chat_img"


Accessibility issue: <img> should be wrapped in a <button> if it's clickable.

mateacademy-ai-mentor · 2026-04-30T17:45:28Z

+                />
+                <div
+                  className="header-chat-name"
+                  onClick={() => setIsChatInfoModalOpen(true)}
+                >
+                  {selectedChat.name || selectedChat.recipient_name || 'Chat'}
+                </div>


Accessibility issue: <div> used for a clickable element. Use a <button> element instead.

mateacademy-ai-mentor · 2026-04-30T17:45:28Z

+                />
+                <div className="send-message-button" onClick={sendMessage}>
+                  &#10148;


Accessibility issue: <div> used for the send button. Use a <button> element instead of <div> with onClick.

mateacademy-ai-mentor · 2026-04-30T17:45:28Z

+    // eslint-disable-next-line no-console
+    return console.error('Connection error:', err.stack);


Socket.IO CORS only allows GET and POST methods (line 14), but the app uses PUT and DELETE HTTP methods. Add 'PUT' and 'DELETE' to the Socket.IO cors methods array to match the Express CORS configuration on line 29.

Check this Luke comment

mateacademy-ai-mentor · 2026-04-30T17:45:28Z

+
+    io.to(chatId.toString()).emit('receive_message', messageData);
+    io.emit('update_chat_list', messageData);


Critical bug - Information leak: Line 262 uses io.emit('update_chat_list', messageData) which broadcasts to ALL connected clients. This reveals chat IDs and message content to users who aren't members of the chat. Change to io.to(chatId.toString()).emit('update_chat_list', messageData) to only notify users in that chat room.

Note: The receive_message event on line 260 correctly uses io.to(chatId.toString()).emit(), but update_chat_list still broadcasts to everyone.

brespect · 2026-05-01T07:03:20Z

+
+const app = express();
+
+const server = http.createServer(app);


separate your server logic using MVC pattern

brespect · 2026-05-01T07:03:43Z

+
+const io = new Server(server, {
+  cors: {
+    origin: 'http://localhost:5173',


better to move this values to .env file, check everywhere

brespect · 2026-05-01T07:04:32Z

+    // eslint-disable-next-line no-console
+    return console.error('Connection error:', err.stack);


Check this Luke comment

brespect

Also, you need to pass all tests and linter checks before requesting the review

nastia7408 · 2026-05-01T12:37:17Z

Я розділила проект на дві папки (client та server) для створення Full Stack чату. І тепер заплуталась. Не можу зрозуміти,
як вирішити цю проблему

solution

5c49b5a

mateacademy-ai-mentor suggested changes Apr 30, 2026

View reviewed changes

solution

22ae79e

nastia7408 requested a review from mateacademy-ai-mentor April 30, 2026 17:42

mateacademy-ai-mentor suggested changes Apr 30, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Apr 30, 2026

View reviewed changes

solution

c7f3f63

nastia7408 requested a review from mateacademy-ai-mentor April 30, 2026 20:23

brespect suggested changes May 1, 2026

View reviewed changes

brespect reviewed May 1, 2026

View reviewed changes

Nastia Mazur added 7 commits May 1, 2026 12:41

solution

b1910c3

solution

967e12b

solution

5715707

solution

3240828

solution

0795f33

solution

164f915

solution

1b7c3a5

nastia7408 closed this by deleting the head repository May 1, 2026


		const messageData = newMessage.rows[0];

		io.to(chatId.toString()).emit('receive_message', messageData);

		// eslint-disable-next-line no-console
		return console.error('Connection error:', err.stack);


		io.to(chatId.toString()).emit('receive_message', messageData);
		io.emit('update_chat_list', messageData);


		const app = express();

		const server = http.createServer(app);

Conversation

nastia7408 commented Apr 30, 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!