-
Notifications
You must be signed in to change notification settings - Fork 266
add solution #198
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?
add solution #198
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| const { rooms } = require('./store'); | ||
| const { broadcastRooms, broadcastToRoom } = require('./utils'); | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In src/index.js you require './src/websocket' from a file that is already in src; this will resolve to src/src/websocket.js and fail. Change the path to './websocket' (or correct relative path) so Node can load the websocket module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect require path: index.js is inside src, but this imports './src/websocket' which resolves to src/src/websocket and will likely throw MODULE_NOT_FOUND. Change to require('./websocket') or the correct relative path to the websocket module. |
||
| function handleMessage(ws, wss, messageAsString) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This repository contains server-side code only. The task requires both client and server and specifically that the username is typed by the user and saved in localStorage (checklist items #1, #3, #4 and #15). Add the client implementation that sends the username to the server and stores it in localStorage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server-side implements room operations and history broadcasting, which is good, but the task requires that the username be saved in localStorage (client-side). Make sure the client saves the username in localStorage and re-sends it when reconnecting/joining a room so the server can set |
||
| const data = JSON.parse(messageAsString); | ||
|
|
||
| switch (data.type) { | ||
| case 'JOIN_ROOM': | ||
| ws.roomId = data.roomId; | ||
| ws.username = data.username; | ||
|
|
||
| if (!rooms[data.roomId]) { | ||
| rooms[data.roomId] = { | ||
| id: data.roomId, | ||
| name: data.roomId, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When creating a missing room inside the JOIN_ROOM handler you set the room |
||
| messages: [], | ||
| }; | ||
| } | ||
|
|
||
| ws.send( | ||
| JSON.stringify({ | ||
| type: 'ROOM_HISTORY', | ||
| roomId: data.roomId, | ||
| messages: rooms[data.roomId].messages, | ||
| }), | ||
| ); | ||
| break; | ||
|
|
||
| case 'NEW_MESSAGE': | ||
| if (ws.roomId && rooms[ws.roomId]) { | ||
| const newMessage = { | ||
| id: Date.now().toString(), | ||
| author: ws.username, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The message author is set to |
||
| time: new Date().toISOString(), | ||
| text: data.text, | ||
| }; | ||
|
|
||
| rooms[ws.roomId].messages.push(newMessage); | ||
|
|
||
| broadcastToRoom(wss, ws.roomId, { | ||
| type: 'MESSAGE', | ||
| roomId: ws.roomId, | ||
| message: newMessage, | ||
| }); | ||
| } | ||
| break; | ||
|
|
||
| case 'CREATE_ROOM': | ||
| const newRoomId = 'room_' + Date.now(); | ||
|
|
||
| rooms[newRoomId] = { id: newRoomId, name: data.roomName, messages: [] }; | ||
| broadcastRooms(wss); | ||
| break; | ||
|
|
||
| case 'RENAME_ROOM': | ||
| if (rooms[data.roomId] && data.roomId !== 'general') { | ||
| rooms[data.roomId].name = data.newName; | ||
| broadcastRooms(wss); | ||
| } | ||
| break; | ||
|
|
||
| case 'DELETE_ROOM': | ||
| if (rooms[data.roomId] && data.roomId !== 'general') { | ||
| delete rooms[data.roomId]; | ||
|
|
||
| wss.clients.forEach((client) => { | ||
| if (client.roomId === data.roomId) { | ||
| client.roomId = 'general'; | ||
| client.send(JSON.stringify({ type: 'ROOM_DELETED' })); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When a room is deleted you set moving clients' |
||
| } | ||
| }); | ||
|
Comment on lines
+83
to
+96
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When deleting a room you set affected clients' roomId to 'general' and send a |
||
| broadcastRooms(wss); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| module.exports = { handleMessage }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,13 @@ | ||
| 'use strict'; | ||
| /* eslint-disable no-console */ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The overall repo is missing client-side implementation files (UI and client JS). The task explicitly requires both client and server; without client code you can't meet requirements like saving username to localStorage, letting users create/rename/join/delete rooms from the UI, and showing previous messages to new users. |
||
| const http = require('http'); | ||
| const { setupWebSocket } = require('./src/websocket'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect require path: this file is already in src, so requiring './src/websocket' will try to load src/src/websocket.js and fail. Change to require('./websocket') or adjust the path to the actual websocket module location. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From src/index.js this require uses './src/websocket' while the file itself is already in src/. That resolves to src/src/websocket which likely doesn't exist. Use a correct relative path such as |
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The repository contains only server-side code (handlers, websocket, utils, store). The task requires a client as well (UI to type username, send it to server, and save username in localStorage). Without client-side code, checklist items #1, #2, #3 and #15 are not fulfilled — add a client that sends JOIN_ROOM/CREATE_ROOM/etc. and saves the username in localStorage. |
||
| const server = http.createServer(); | ||
|
|
||
| setupWebSocket(server); | ||
|
|
||
| const PORT = 8080; | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server stores |
||
| server.listen(PORT, () => { | ||
| console.log(`Server is listening on port ${PORT}`); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| const rooms = { | ||
| general: { id: 'general', name: 'General', messages: [] }, | ||
| }; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This require is incorrect given this file is already in src. Requiring './src/websocket' will try to load src/src/websocket and fail with MODULE_NOT_FOUND. Change to require('./websocket') (or adjust path relative to this file). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This require is incorrect given this file is already in src. Requiring './src/websocket' will try to load src/src/websocket and fail with MODULE_NOT_FOUND. Change to require('./websocket') (or adjust path relative to this file). |
||
|
|
||
| module.exports = { rooms }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| const WebSocket = require('ws'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The task requires both client and server parts and that the username is saved in localStorage and sent to the server. I don't see any client-side files here implementing the UI, localStorage usage, or sending username/join/create room actions. You must add the client implementation (HTML/JS) so users can type a username, save it to localStorage, and perform room actions. |
||
| const { rooms } = require('./store'); | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This require path is incorrect given file location. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This require path is incorrect for a file located at src/index.js: requiring './src/websocket' will try to load src/src/websocket.js and likely fail. Use the correct relative path (e.g. |
||
| function broadcastRooms(wss) { | ||
| const roomList = Object.values(rooms).map((room) => ({ | ||
| id: room.id, | ||
| name: room.name, | ||
| })); | ||
| const message = JSON.stringify({ type: 'ROOMS_LIST', rooms: roomList }); | ||
|
|
||
|
Comment on lines
+8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The server expects the client to send |
||
| wss.clients.forEach((client) => { | ||
| if (client.readyState === WebSocket.OPEN) { | ||
| client.send(message); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| function broadcastToRoom(wss, roomId, messageObj) { | ||
|
Comment on lines
+12
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When JOIN_ROOM creates a new room (if it doesn't exist) the code doesn't broadcast the updated rooms list to other clients. That means other connected users won't see rooms that are implicitly created by JOIN_ROOM. Consider calling |
||
| const message = JSON.stringify(messageObj); | ||
|
|
||
| wss.clients.forEach((client) => { | ||
| if (client.readyState === WebSocket.OPEN && client.roomId === roomId) { | ||
| client.send(message); | ||
| } | ||
|
Comment on lines
+18
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When JOIN_ROOM auto-creates a room (inside this block) you also need to notify other connected clients about the new room. Call |
||
| }); | ||
| } | ||
|
|
||
| module.exports = { broadcastRooms, broadcastToRoom }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| const WebSocket = require('ws'); | ||
| const { rooms } = require('./store'); | ||
| const { handleMessage } = require('./handlers'); | ||
|
|
||
| function setupWebSocket(server) { | ||
| const wss = new WebSocket.Server({ server }); | ||
|
|
||
| const interval = setInterval(() => { | ||
| wss.clients.forEach((ws) => { | ||
| if (ws.isAlive === false) { | ||
| return ws.terminate(); | ||
| } | ||
|
|
||
| ws.isAlive = false; | ||
| ws.ping(); | ||
| }); | ||
| }, 30000); | ||
|
|
||
| wss.on('close', function close() { | ||
| clearInterval(interval); | ||
| }); | ||
|
|
||
| wss.on('connection', (ws) => { | ||
| ws.isAlive = true; | ||
|
|
||
| ws.on('pong', () => { | ||
| ws.isAlive = true; | ||
| }); | ||
|
|
||
| ws.send( | ||
| JSON.stringify({ | ||
| type: 'ROOMS_LIST', | ||
| rooms: Object.values(rooms).map((r) => ({ id: r.id, name: r.name })), | ||
| }), | ||
| ); | ||
|
Comment on lines
+20
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On new connections you send the rooms list but do not assign a default room or send its history. Consider setting a default
Comment on lines
+20
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On new connections you send the rooms list but do not assign a default room or send its history. Consider setting a default |
||
|
|
||
| ws.on('message', (message) => handleMessage(ws, wss, message)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling |
||
|
|
||
| ws.on('close', () => { | ||
| ws.roomId = null; | ||
| }); | ||
| }); | ||
|
|
||
| return wss; | ||
| } | ||
|
|
||
| module.exports = { setupWebSocket }; | ||
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.
The project currently lacks the client-side implementation required by the task: UI for typing a username, saving it to localStorage, sending it to the server, and room UI (create/rename/join/delete). These are mandatory according to the description and must be added.