-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Implemented the who is typing feature #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds 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
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
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 detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 ? (...) : nullwithusers?.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.
⛔ Files ignored due to path filters (2)
client/package-lock.jsonis excluded by!**/package-lock.jsonserver/package-lock.jsonis 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.dataonly 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 | |||
There was a problem hiding this comment.
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.
| 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]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
| 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"); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
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.
| 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]); |
| 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 | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
| <Input | ||
| message={message} | ||
| setMessage={setMessage} | ||
| sendMessage={sendMessage} | ||
| socket={socket} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| <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).
| 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); | ||
| }; |
There was a problem hiding this comment.
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.
| 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.
| <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)} | ||
| /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| <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).
| // ✅ 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', ''); | ||
| }); |
There was a problem hiding this comment.
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.
| // ✅ 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.
| // 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(); | ||
| }); |
There was a problem hiding this comment.
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.
| // 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).
Hi, I implemented who is typing features, much like in a whatsapp group
Summary by CodeRabbit
New Features
Chores