Skip to content

Apply socket io#122

Open
hassanelnajjar wants to merge 2 commits intomainfrom
apply-socket-io
Open

Apply socket io#122
hassanelnajjar wants to merge 2 commits intomainfrom
apply-socket-io

Conversation

@hassanelnajjar
Copy link
Copy Markdown
Collaborator

Apply socket io

  • Apply Socket.io to Booking Form So when patients book an appointment on the current day, it will re-render the appointmentTable of today Schedule.

@hassanelnajjar hassanelnajjar self-assigned this Apr 21, 2021
@AlaaShurrab AlaaShurrab self-requested a review April 21, 2021 12:48
io.on('connection', (socket) => {
socket.on('add appointment', (value) => {
if (new Date(value).toDateString() === new Date().toDateString()) {
io.sockets.emit('updateAppointments');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why did you use io.sockets.emit while you can use io.emit directly

const io = socketIO(server);
io.on('connection', (socket) => {
socket.on('add appointment', (value) => {
if (new Date(value).toDateString() === new Date().toDateString()) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this condition although it is not that important here, is easy to bypass
but for other socket requests, you should make the io route to what the normal route do with the same amount of validation

import { useLocation } from 'react-router-dom';
import { bool } from 'prop-types';
import { message } from 'antd';
import socketIOClient from 'socket.io-client';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

repeated code
isn't it better to create a folder IO and export the functions from it

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

Labels

Awaiting Review enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants