NW6 | Fikret Ellek| Module Servers | Stretch challenges | Week 2 #181
Conversation
|
react app ---> https://fe-chat-react-app.netlify.app/ |
musayuksel
left a comment
musayuksel
left a comment
There was a problem hiding this comment.
I loved your design and solution! Great job @fikretellek!
I've added some improvements that you might consider.
| useEffect(() => { | ||
| getMessages(); | ||
| const interval = setInterval(() => getMessages(), 10000); | ||
| return () => clearInterval(interval); | ||
| }, []); |
There was a problem hiding this comment.
Great job! Efficiently using the useEffect and clearInterval
There was a problem hiding this comment.
I am planning to use websocket to achieve real time communication. I'll try to implement it today. Can we have a meeting about websockets if you have time? I quite didn't understand it 😆
| }) | ||
| .then((response) => response.json()) | ||
| .then((data) => setMessages(data.slice().reverse())); | ||
| } |
There was a problem hiding this comment.
Two suggestions for improvement:
First : consider using state to store the values of the name and message inputs.
Second : extract the fetch functionality into its own function to improve code organisation and maintain single responsibility.
There was a problem hiding this comment.
I didn't understand the second suggestion. could you please make it more clear?
There was a problem hiding this comment.
Well, we can create a single reusable fetch function to handle the fetching logic, like:
function fetchMessages(url, options) {
return fetch(url, options)
.then((response) => response.json())
.then((data) => data.slice().reverse());
}
then you can use it for both getMessages and setMessages:
function getMessages() {
fetchMessages("https://module-servers.onrender.com/messages")
.then((data) => setMessages(data));
}
//...
const requestOptions = {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(newMessage),
};
fetchMessages("https://module-servers.onrender.com/messages", requestOptions)
.then((data) => setMessages(data));
There are different ways to do that, you can update it according to your taste ;)
chat-react-app/src/App.jsx
Outdated
| </div> | ||
| <div id="messages"> | ||
| {messages.map((message, index) => { | ||
| const currentUser = document.getElementById("name").value; |
…n for generating messages implemented
|
2 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.