Skip to content

Conversation

@mnatesan18
Copy link

@mnatesan18 mnatesan18 commented Aug 21, 2025

Hi, I implemented who is typing features, much like in a whatsapp group

Summary by CodeRabbit

  • New Features

    • Real-time typing indicators in chat: see when others are typing and indicators auto-clear on send or after inactivity.
  • Chores

    • Added client API URL environment variable (defaults to localhost).
    • Updated dev/build/test scripts to run with OpenSSL legacy provider.
    • Removed outdated repository funding entry and cleared obsolete deployment state.

@coderabbitai
Copy link

coderabbitai bot commented Aug 21, 2025

Walkthrough

Adds a client env var and local ENDPOINT, updates react-scripts commands to use --openssl-legacy-provider, implements typing indicator (client emits typing/stopTyping; server broadcasts showTyping), updates Chat and Input to handle typing and socket prop, and extends server socket join/disconnect handling and roomData updates.

Changes

Cohort / File(s) Summary
Client env & endpoint
client/.env, client/src/App.js
Adds REACT_APP_API_URL=http://localhost:5000 and an ENDPOINT constant (localhost).
Client build scripts
client/package.json
Prepends --openssl-legacy-provider to start, build, and test script invocations.
Chat UI & typing styles
client/src/components/Chat/Chat.js, client/src/components/Chat/Chat.css
Adds typing state, listens for showTyping, renders typing indicator, passes socket to Input, cleans up listeners; adds .typing CSS class.
Input typing support
client/src/components/Input/Input.js
Component signature now accepts socket; emits typing and stopTyping with inactivity timer; uses form onSubmit for sending.
Server socket behavior
server/index.js
Stores user info on socket.data, adds typing and stopTyping handlers broadcasting showTyping, refactors join/disconnect flows to emit welcome/join/left messages and updated roomData.
Repo housekeeping
.github/FUNDING.yml, .netlify/state.json, server/package.json
Removed a funding line, deleted Netlify state file, and minor whitespace change in server package.json.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor UserA as Client A
  participant Input as Input (client)
  participant Chat as Chat UI
  participant Srv as Socket.IO Server
  participant Room as Other Clients

  rect rgba(230,245,255,0.6)
    UserA->>Srv: connect & join({name,room})
    Srv->>UserA: welcome
    Srv-->>Room: broadcast join + roomData
  end

  rect rgba(240,255,240,0.6)
    Note over Input,Chat: typing lifecycle
    Input->>Srv: typing
    Srv-->>Room: showTyping({name})
    Room->>Chat: show "name is typing…"
  end

  rect rgba(255,250,230,0.6)
    Input->>Srv: stopTyping (after timeout or on send)
    Srv-->>Room: clear showTyping
  end

  rect rgba(245,240,255,0.6)
    Chat->>Srv: message({text})
    Srv-->>Room: message({user,text,time})
    Srv-->>UserA: message echo
  end

  rect rgba(255,235,238,0.6)
    UserA-->>Srv: disconnect
    Srv-->>Room: left message + roomData
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit types with nimble paw,
Sends "typing" signals that we saw.
The server hums, the room replies,
Then quiet falls—message arrives.
Hops, scripts, and endpoints all align—🐇✨

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8119ad7 and 448b77f.

📒 Files selected for processing (5)
  • .github/FUNDING.yml (0 hunks)
  • .netlify/state.json (0 hunks)
  • client/src/App.js (1 hunks)
  • server/index.js (1 hunks)
  • server/package.json (1 hunks)
💤 Files with no reviewable changes (2)
  • .github/FUNDING.yml
  • .netlify/state.json
✅ Files skipped from review due to trivial changes (1)
  • server/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
  • client/src/App.js
  • server/index.js
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (12)
client/.env (1)

1-1: Be mindful: .env is committed — don’t place secrets here.

CRA inlines REACT_APP_* at build time. Keeping a default URL here is fine, but never put secrets in this file. If needed, provide a committed .env.example and use .env.local for machine-specific overrides.

CRA note on exposure of REACT_APP_* vars. (create-react-app.dev)

client/package.json (1)

16-18: The --openssl-legacy-provider workaround may be brittle; prefer NODE_OPTIONS or upgrading react-scripts.

Placing --openssl-legacy-provider before the subcommand can be flaky across environments. Two more robust options:

  • Short term: set NODE_OPTIONS in the script (use cross-env for Windows).
  • Long term: upgrade to react-scripts 5 which drops the need for this workaround (or move off CRA).

Example using cross-env (requires adding it as a devDependency):

   "scripts": {
-    "start": "react-scripts --openssl-legacy-provider start",
-    "build": "react-scripts --openssl-legacy-provider build",
-    "test": "react-scripts --openssl-legacy-provider test",
+    "start": "cross-env NODE_OPTIONS=--openssl-legacy-provider react-scripts start",
+    "build": "cross-env NODE_OPTIONS=--openssl-legacy-provider react-scripts build",
+    "test": "cross-env NODE_OPTIONS=--openssl-legacy-provider react-scripts test",
     "eject": "react-scripts eject"
   }

Context on this workaround and CRA/node/OpenSSL friction. (github.com, stackoverflow.com)

server/index.js (3)

16-16: Prefer the canonical 'connection' event for readability.

io.on('connection') is the documented event name on the server side; 'connect' is an alias but less common in docs. Using 'connection' improves clarity for future readers.

-io.on('connect', (socket) => {
+io.on('connection', (socket) => {

Docs use "connection" consistently. (socket.io)


74-84: Consistent 'admin' casing; optionally clear typing state on disconnect.

Earlier messages use user: 'admin', but disconnect uses 'Admin'. Standardize to one value to keep client-side styling/logic consistent. Also consider emitting showTyping '' on disconnect to clear stale indicators.

 if (user) {
-  io.to(user.room).emit('message', {
-    user: 'Admin',
-    text: `${user.name} has left.`
-  });
+  io.to(user.room).emit('message', { user: 'admin', text: `${user.name} has left.` });
+  // Clear any typing indicator that might be shown for this user
+  io.to(user.room).emit('showTyping', '');
   io.to(user.room).emit('roomData', {
     room: user.room,
     users: getUsersInRoom(user.room)
   });
 }

87-89: Minor: include port in the startup log.

Helps during local dev and deployments.

-server.listen(process.env.PORT || 5000, () =>
-  console.log(`Server has started.`)
-);
+const PORT = process.env.PORT || 5000;
+server.listen(PORT, () => console.log(`Server started on port ${PORT}`));
client/src/components/Input/Input.js (2)

1-1: Remove unused import.

useEffect isn’t used (yet). Either remove it or add a cleanup effect (see next comment).

-import React, { useEffect, useRef } from "react";
+import React, { useRef } from "react";

24-36: Optional: disable Send when input is empty; improves UX and prevents accidental submits.

This mirrors the sendMessage guard in Chat.js.

-  <button className="sendButton" type="submit">
+  <button className="sendButton" type="submit" disabled={!message?.trim()}>
     Send
   </button>
client/src/App.js (1)

7-7: Remove redundant ENDPOINT here or centralize config.

This constant isn’t used in this file and duplicates the value hard-coded in Chat.js. Prefer a single source of truth via REACT_APP_API_URL, or export it from a config module. If you keep the env var approach, you can safely delete this line.

-const ENDPOINT = 'http://localhost:5000';
client/src/components/Chat/Chat.js (4)

12-14: Centralize ENDPOINT and avoid diverging client configs.

Hard-coding the URL here duplicates App-level/ENV config. Use the env var with a sensible fallback to keep dev/prod consistent.

-const ENDPOINT = "http://localhost:5000";
+const ENDPOINT = process.env.REACT_APP_API_URL || "http://localhost:5000";

Follow-up:

  • Make sure client/.env has REACT_APP_API_URL set for local and deployed environments.
  • Consider moving this to a small config module (e.g., client/src/config.js) and import where needed to avoid drift.

73-75: Typing indicator UX: consider multi-user and auto-expiry.

The single string will be overwritten if multiple people type. Consider tracking a set of typers and rendering “Alice and Bob are typing…”, and auto-clearing after a heartbeat if no updates arrive.

If helpful, I can draft a minimal client-side aggregator that maintains a Map<name, lastSeenMs> and prunes stale entries.


16-22: Users array + TextContainer condition.

users is initialized as an array, which is always truthy. TextContainer currently renders the “People currently chatting:” section even when the list is empty. Consider showing it only when users.length > 0 to avoid an empty section.

For example (in TextContainer):

  • Replace users ? (...) : null with users?.length ? (...) : null.

2-2: Optional: Drop query-string for native URLSearchParams.

You can remove an extra dependency by using the platform API:

const params = new URLSearchParams(location.search);
const name = params.get("name") || "";
const room = params.get("room") || "";
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c0b49d1 and 8119ad7.

⛔ Files ignored due to path filters (2)
  • client/package-lock.json is excluded by !**/package-lock.json
  • server/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • client/.env (1 hunks)
  • client/package.json (1 hunks)
  • client/src/App.js (1 hunks)
  • client/src/components/Chat/Chat.css (1 hunks)
  • client/src/components/Chat/Chat.js (1 hunks)
  • client/src/components/Input/Input.js (1 hunks)
  • server/index.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
client/src/App.js (1)
client/src/components/Chat/Chat.js (1)
  • ENDPOINT (12-12)
client/src/components/Input/Input.js (1)
client/src/components/Chat/Chat.js (3)
  • socket (13-13)
  • sendMessage (58-65)
  • message (19-19)
server/index.js (1)
server/users.js (5)
  • addUser (3-17)
  • user (12-12)
  • getUsersInRoom (27-27)
  • getUser (25-25)
  • removeUser (19-23)
client/src/components/Chat/Chat.js (3)
client/src/App.js (1)
  • ENDPOINT (7-7)
client/src/components/Input/Input.js (1)
  • Input (5-38)
client/src/components/TextContainer/TextContainer.js (1)
  • TextContainer (7-34)
🔇 Additional comments (4)
client/src/components/Chat/Chat.css (1)

8-13: LGTM: minimal, readable styling for the typing indicator.

The .typing class is clear and non-intrusive. No layout regressions are expected.

server/index.js (1)

25-27: No action needed—Socket.IO versions align

  • Both server and client are using Socket.IO v2.2.0 (^2.2.0), so there is no major‐version mismatch.
  • While socket.data only became a first-class API in v4, assigning to it still works as a plain property in v2.x.
client/src/components/Input/Input.js (1)

23-36: Sanity check passed: only one Enter handler
I searched the entire client/src directory for any onKeyPress/onKeyDown handlers that reference the Enter key and found only the instance in Input.js. There are no other potential double-submit handlers to address.

client/src/components/Chat/Chat.js (1)

38-40: Nice use of functional state updates and cleanup.

Using functional updates for messages prevents stale closures. Also good to see explicit socket.off calls in cleanup—once the dependency fix is applied, this will keep listeners tidy.

Also applies to: 51-55

@@ -0,0 +1 @@
REACT_APP_API_URL=http://localhost:5000
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Avoid hardcoding the API URL in code; rely on REACT_APP_API_URL everywhere.

Since this file now defines REACT_APP_API_URL, ensure the client uses process.env.REACT_APP_API_URL instead of hardcoded "http://localhost:5000" (the AI summary notes an ENDPOINT constant in App.js). This keeps environments decoupled and avoids drift. Also consider adding .env.development and .env.production variants for clean overrides.

Reference: CRA env var conventions and caveats. (create-react-app.dev)

Run to find hardcoded endpoints:


🏁 Script executed:

#!/bin/bash
# Find hardcoded localhost endpoints in the client
rg -nP --type=js --type=jsx --type=ts --type=tsx -C2 'https?://localhost:5000|process\.env\.REACT_APP_API_URL' client/

Length of output: 148


🏁 Script executed:

#!/bin/bash
# Search for hardcoded localhost endpoints in client source files
rg -n -C2 'https?://localhost:5000' client/ --glob '*.js' --glob '*.jsx' --glob '*.ts' --glob '*.tsx'

# Search for usage of the environment variable
rg -n -C2 'process\.env\.REACT_APP_API_URL' client/ --glob '*.js' --glob '*.jsx' --glob '*.ts' --glob '*.tsx'

Length of output: 716


Remove hardcoded API endpoints and switch to environment variables

The client currently hardcodes the API URL in two places:

  • client/src/App.js (line 7)
  • client/src/components/Chat/Chat.js (line 12)

Replace

const ENDPOINT = 'http://localhost:5000';

with

const ENDPOINT = process.env.REACT_APP_API_URL;

(you may optionally include a fallback, e.g.

const ENDPOINT = process.env.REACT_APP_API_URL || 'http://localhost:5000';

)

Additionally, create environment-specific files—.env.development and .env.production—to define REACT_APP_API_URL per environment, following the Create React App conventions: https://create-react-app.dev/docs/adding-custom-environment-variables. This ensures each deployment uses the correct API endpoint and prevents configuration drift.

🤖 Prompt for AI Agents
In client/.env around line 1 and in files client/src/App.js (line 7) and
client/src/components/Chat/Chat.js (line 12), remove the hardcoded ENDPOINT and
replace it with reading from the environment (process.env.REACT_APP_API_URL with
an optional fallback to the localhost URL); add environment-specific files
.env.development and .env.production at the project root defining
REACT_APP_API_URL for each environment per Create React App conventions, update
the two JS files to use that env var, and restart the dev/build process so the
new variables are picked up.

Comment on lines 23 to +35
useEffect(() => {
const { name, room } = queryString.parse(location.search);

socket = io(ENDPOINT);

setRoom(room);
setName(name)
setName(name);

socket.emit('join', { name, room }, (error) => {
if(error) {
socket.emit("join", { name, room }, (error) => {
if (error) {
alert(error);
}
});
}, [ENDPOINT, location.search]);
}, [location.search]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Disconnect the socket on cleanup to prevent ghost users and leaked connections.

When the component unmounts or the room changes (location.search), the socket remains connected. The server won’t receive a disconnect, leaving “stuck” users in roomData and continuing to broadcast typing/messages to an unmounted UI.

   useEffect(() => {
     const { name, room } = queryString.parse(location.search);
     socket = io(ENDPOINT);

     setRoom(room);
     setName(name);

     socket.emit("join", { name, room }, (error) => {
       if (error) {
         alert(error);
       }
     });
-  }, [location.search]);
+    return () => {
+      try {
+        socket?.emit("stopTyping");
+      } catch (_) {}
+      // Ensure the server gets a proper disconnect and the client frees resources.
+      socket?.disconnect();
+    };
+  }, [location.search]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
const { name, room } = queryString.parse(location.search);
socket = io(ENDPOINT);
setRoom(room);
setName(name)
setName(name);
socket.emit('join', { name, room }, (error) => {
if(error) {
socket.emit("join", { name, room }, (error) => {
if (error) {
alert(error);
}
});
}, [ENDPOINT, location.search]);
}, [location.search]);
useEffect(() => {
const { name, room } = queryString.parse(location.search);
socket = io(ENDPOINT);
setRoom(room);
setName(name);
socket.emit("join", { name, room }, (error) => {
if (error) {
alert(error);
}
});
return () => {
try {
socket?.emit("stopTyping");
} catch (_) {}
// Ensure the server gets a proper disconnect and the client frees resources.
socket?.disconnect();
};
}, [location.search]);
🤖 Prompt for AI Agents
In client/src/components/Chat/Chat.js around lines 23 to 35, the socket is
created on mount/when location.search changes but never cleaned up; add a
cleanup function returned from the useEffect that removes listeners and fully
disconnects the socket (e.g., socket.off() for handlers you registered and
socket.disconnect() or socket.emit('disconnect') followed by socket.close()),
and also clear any module-level socket reference if present to avoid reused
stale sockets when the component remounts or room changes.

Comment on lines 37 to +56
useEffect(() => {
socket.on('message', message => {
setMessages(messages => [ ...messages, message ]);
socket.on("message", (message) => {
setMessages((msgs) => [...msgs, message]);
});

socket.on("roomData", ({ users }) => {
setUsers(users);
});
}, []);

// ✅ Handle typing events
socket.on("showTyping", (msg) => {
setTyping(msg);
});

return () => {
socket.off("message");
socket.off("roomData");
socket.off("showTyping");
};
}, []);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Re-subscribe listeners on room change; avoid stale socket references.

This effect uses an empty dependency array, so it won’t re-run if a new socket is created on a room change. That leaves the new socket without listeners, breaking message updates and typing indicators. Also, pass explicit handler refs to off().

-  useEffect(() => {
-    socket.on("message", (message) => {
-      setMessages((msgs) => [...msgs, message]);
-    });
-
-    socket.on("roomData", ({ users }) => {
-      setUsers(users);
-    });
-
-    // ✅ Handle typing events
-    socket.on("showTyping", (msg) => {
-      setTyping(msg);
-    });
-
-    return () => {
-      socket.off("message");
-      socket.off("roomData");
-      socket.off("showTyping");
-    };
-  }, []);
+  useEffect(() => {
+    if (!socket) return;
+
+    const onMessage = (message) => setMessages((msgs) => [...msgs, message]);
+    const onRoomData = ({ users }) => setUsers(users);
+    const onShowTyping = (msg) => setTyping(msg);
+
+    socket.on("message", onMessage);
+    socket.on("roomData", onRoomData);
+    socket.on("showTyping", onShowTyping);
+
+    return () => {
+      socket.off("message", onMessage);
+      socket.off("roomData", onRoomData);
+      socket.off("showTyping", onShowTyping);
+    };
+  }, [location.search]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
socket.on('message', message => {
setMessages(messages => [ ...messages, message ]);
socket.on("message", (message) => {
setMessages((msgs) => [...msgs, message]);
});
socket.on("roomData", ({ users }) => {
setUsers(users);
});
}, []);
// ✅ Handle typing events
socket.on("showTyping", (msg) => {
setTyping(msg);
});
return () => {
socket.off("message");
socket.off("roomData");
socket.off("showTyping");
};
}, []);
useEffect(() => {
if (!socket) return;
const onMessage = (message) =>
setMessages((msgs) => [...msgs, message]);
const onRoomData = ({ users }) =>
setUsers(users);
const onShowTyping = (msg) =>
setTyping(msg);
socket.on("message", onMessage);
socket.on("roomData", onRoomData);
socket.on("showTyping", onShowTyping);
return () => {
socket.off("message", onMessage);
socket.off("roomData", onRoomData);
socket.off("showTyping", onShowTyping);
};
}, [location.search]);

Comment on lines 58 to +65
const sendMessage = (event) => {
event.preventDefault();

if(message) {
socket.emit('sendMessage', message, () => setMessage(''));
if (message) {
socket.emit("sendMessage", message, () => setMessage(""));
socket.emit("stopTyping"); // stop typing after sending
}
}
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Guard against undefined socket and trim messages.

On fast interactions, sendMessage can run before the socket is ready; also avoid sending whitespace messages.

   const sendMessage = (event) => {
     event.preventDefault();

-    if (message) {
-      socket.emit("sendMessage", message, () => setMessage(""));
-      socket.emit("stopTyping"); // stop typing after sending
-    }
+    if (!socket) return;
+    const text = message.trim();
+    if (text) {
+      socket.emit("sendMessage", text, () => setMessage(""));
+      socket.emit("stopTyping"); // stop typing after sending
+    }
   };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sendMessage = (event) => {
event.preventDefault();
if(message) {
socket.emit('sendMessage', message, () => setMessage(''));
if (message) {
socket.emit("sendMessage", message, () => setMessage(""));
socket.emit("stopTyping"); // stop typing after sending
}
}
};
const sendMessage = (event) => {
event.preventDefault();
if (!socket) return;
const text = message.trim();
if (text) {
socket.emit("sendMessage", text, () => setMessage(""));
socket.emit("stopTyping"); // stop typing after sending
}
};
🤖 Prompt for AI Agents
In client/src/components/Chat/Chat.js around lines 58 to 65, sendMessage can be
called before socket is initialized and may send whitespace-only messages;
update the handler to trim the message first and only proceed if the trimmed
message is non-empty, and guard all socket.emit calls with a check that socket
is defined (or connected) so emits (sendMessage and stopTyping) are only invoked
when socket exists; keep event.preventDefault() as-is and clear the input via
setMessage("") after a successful emit.

Comment on lines +76 to +81
<Input
message={message}
setMessage={setMessage}
sendMessage={sendMessage}
socket={socket}
/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Render Input only when socket is ready to avoid runtime errors in Input.

Input emits typing on change; if socket is not yet initialized, it can throw. Render-gate on socket readiness.

-        <Input
-          message={message}
-          setMessage={setMessage}
-          sendMessage={sendMessage}
-          socket={socket}
-        />
+        {socket && (
+          <Input
+            message={message}
+            setMessage={setMessage}
+            sendMessage={sendMessage}
+            socket={socket}
+          />
+        )}

Optional: Alternatively, add a null-check inside Input’s handleChange before emitting.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Input
message={message}
setMessage={setMessage}
sendMessage={sendMessage}
socket={socket}
/>
{socket && (
<Input
message={message}
setMessage={setMessage}
sendMessage={sendMessage}
socket={socket}
/>
)}
🤖 Prompt for AI Agents
In client/src/components/Chat/Chat.js around lines 76 to 81, the Input component
can call socket.emit during typing before socket is initialized causing runtime
errors; fix by rendering-gating Input only when socket is ready (e.g., render
Input only if socket != null and socket.connected is true or socket.readyState
indicates open) so it won't mount without a valid socket, or alternatively add a
defensive null-check in Input’s change handler before calling socket.emit
(return early if socket is null/undefined).

Comment on lines +8 to +21
const handleChange = (e) => {
setMessage(e.target.value);

// Emit typing event
socket.emit("typing");

// Clear old timeout
if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current);

// After 1.5s of no typing, send stopTyping
typingTimeoutRef.current = setTimeout(() => {
socket.emit("stopTyping");
}, 1500);
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Guard socket emits and add a cleanup for the typing timer.

Avoid exceptions when socket is null/not connected, and clear the timer on unmount to prevent orphaned timeouts.

 const handleChange = (e) => {
   setMessage(e.target.value);

   // Emit typing event
-  socket.emit("typing");
+  if (socket) socket.emit("typing");

   // Clear old timeout
   if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current);

   // After 1.5s of no typing, send stopTyping
   typingTimeoutRef.current = setTimeout(() => {
-    socket.emit("stopTyping");
+    if (socket) socket.emit("stopTyping");
   }, 1500);
 };

+// Optional: clear timer on unmount
+// React 16.9 supports this pattern
+// useEffect(() => () => { if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current); }, []);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleChange = (e) => {
setMessage(e.target.value);
// Emit typing event
socket.emit("typing");
// Clear old timeout
if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current);
// After 1.5s of no typing, send stopTyping
typingTimeoutRef.current = setTimeout(() => {
socket.emit("stopTyping");
}, 1500);
};
const handleChange = (e) => {
setMessage(e.target.value);
// Emit typing event
if (socket) socket.emit("typing");
// Clear old timeout
if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current);
// After 1.5s of no typing, send stopTyping
typingTimeoutRef.current = setTimeout(() => {
if (socket) socket.emit("stopTyping");
}, 1500);
};
// Optional: clear timer on unmount
// React 16.9 supports this pattern
// useEffect(() => () => {
// if (typingTimeoutRef.current) clearTimeout(typingTimeoutRef.current);
// }, []);
🤖 Prompt for AI Agents
In client/src/components/Input/Input.js around lines 8 to 21, the code emits
"typing" and "stopTyping" without checking socket and sets a timeout that isn't
cleared on unmount; update handleChange to guard socket emits (e.g., if (socket
&& socket.emit) socket.emit(...)) to avoid exceptions when socket is
null/disconnected, and add a useEffect cleanup that clears
typingTimeoutRef.current (if present) on unmount to prevent orphaned timeouts;
also ensure you clear any existing timeout before setting a new one as currently
implemented.

Comment on lines +24 to +32
<form className="form" onSubmit={sendMessage}>
<input
className="input"
type="text"
placeholder="Type a message..."
value={message}
onChange={handleChange}
onKeyPress={(e) => (e.key === "Enter" ? sendMessage(e) : null)}
/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix double-submit: Enter triggers both onKeyPress and form onSubmit.

As written, pressing Enter will call sendMessage twice (once from onKeyPress and once from form submit), which can duplicate messages. Drop onKeyPress and rely on the form’s onSubmit.

   <input
     className="input"
     type="text"
     placeholder="Type a message..."
     value={message}
     onChange={handleChange}
-    onKeyPress={(e) => (e.key === "Enter" ? sendMessage(e) : null)}
   />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<form className="form" onSubmit={sendMessage}>
<input
className="input"
type="text"
placeholder="Type a message..."
value={message}
onChange={handleChange}
onKeyPress={(e) => (e.key === "Enter" ? sendMessage(e) : null)}
/>
<form className="form" onSubmit={sendMessage}>
<input
className="input"
type="text"
placeholder="Type a message..."
value={message}
onChange={handleChange}
/>
🤖 Prompt for AI Agents
In client/src/components/Input/Input.js around lines 24 to 32, the input has an
onKeyPress that calls sendMessage which combined with the form's onSubmit causes
double submission; remove the onKeyPress prop from the input so Enter only
triggers the form onSubmit, and verify sendMessage called by onSubmit still
prevents default and handles the input state (no other duplicate call sites
remain).

Comment on lines +46 to +56
// ✅ User typing
socket.on('typing', () => {
const { name, room } = socket.data;
socket.to(room).emit('showTyping', `${name} is typing...`);
});

// ✅ User stopped typing
socket.on('stopTyping', () => {
const { room } = socket.data;
socket.to(room).emit('showTyping', '');
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Harden typing/stopTyping against pre-join races.

If a client emits typing before join finishes, socket.data may be undefined, causing a crash. Guard and no-op when room isn’t available.

 socket.on('typing', () => {
-  const { name, room } = socket.data;
-  socket.to(room).emit('showTyping', `${name} is typing...`);
+  const data = socket.data || {};
+  if (!data.room) return;
+  socket.to(data.room).emit('showTyping', `${data.name} is typing...`);
 });

 socket.on('stopTyping', () => {
-  const { room } = socket.data;
-  socket.to(room).emit('showTyping', '');
+  const data = socket.data || {};
+  if (!data.room) return;
+  socket.to(data.room).emit('showTyping', '');
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// ✅ User typing
socket.on('typing', () => {
const { name, room } = socket.data;
socket.to(room).emit('showTyping', `${name} is typing...`);
});
// ✅ User stopped typing
socket.on('stopTyping', () => {
const { room } = socket.data;
socket.to(room).emit('showTyping', '');
});
// ✅ User typing
socket.on('typing', () => {
const data = socket.data || {};
if (!data.room) return;
socket.to(data.room).emit('showTyping', `${data.name} is typing...`);
});
// ✅ User stopped typing
socket.on('stopTyping', () => {
const data = socket.data || {};
if (!data.room) return;
socket.to(data.room).emit('showTyping', '');
});
🤖 Prompt for AI Agents
In server/index.js around lines 46 to 56, the typing and stopTyping handlers
assume socket.data (and its room/name) always exist which can crash if a client
emits before join completes; guard access by checking socket.data and the room
(and name for typing) and return early (no-op) when room is missing; for typing
also ensure name is present before emitting, and avoid emitting if guards fail
so pre-join races are safe.

Comment on lines +58 to 68
// User sends message
socket.on('sendMessage', (message, callback) => {
const user = getUser(socket.id);

io.to(user.room).emit('message', { user: user.name, text: message });
io.to(user.room).emit('message', {
user: user.name,
text: message
});

callback();
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Guard sendMessage against missing user; ack errors explicitly.

If getUser(socket.id) returns undefined, io.to(user.room) will throw. Return an error via callback instead.

 socket.on('sendMessage', (message, callback) => {
   const user = getUser(socket.id);

-  io.to(user.room).emit('message', {
-    user: user.name,
-    text: message
-  });
+  if (!user) return callback && callback('User not found');
+  io.to(user.room).emit('message', { user: user.name, text: message });
   callback();
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// User sends message
socket.on('sendMessage', (message, callback) => {
const user = getUser(socket.id);
io.to(user.room).emit('message', { user: user.name, text: message });
io.to(user.room).emit('message', {
user: user.name,
text: message
});
callback();
});
// User sends message
socket.on('sendMessage', (message, callback) => {
const user = getUser(socket.id);
if (!user) return callback && callback('User not found');
io.to(user.room).emit('message', { user: user.name, text: message });
callback();
});
🤖 Prompt for AI Agents
In server/index.js around lines 58 to 68, the sendMessage handler assumes
getUser(socket.id) always returns a user and will throw if undefined; add a
guard that checks if user is present, and if not call the provided callback with
an error (e.g., callback({ error: 'User not found' })) and return early; if the
user exists proceed to emit the message to user.room and then call callback with
success (or no args).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant