Improve the chat example with more ES6 features #3240
Conversation
sergiolenza
changed the title
examples/chat/public/main.js
Outdated
| if (typeof options.prepend === 'undefined') { | ||
| options.prepend = false; | ||
| } | ||
| const addMessageElement = (el, options = { fade: true, prepend: false }) => { |
|
@sergiolenza could you please review the comments made to the PR? Thanks a lot! |
Yes of course @darrachequesne. |
| let message = ''; | ||
| if (data.numUsers === 1) { | ||
| message += "there's 1 participant"; | ||
| message += `there's 1 participant`; |
There was a problem hiding this comment.
The string here should be single quotes, since there's no variable being insert (following the style of the rest of the page).
| message += `there's 1 participant`; | |
| message += 'there's 1 participant'; |
| } | ||
|
|
||
| // Adds the visual chat message to the message list | ||
| const addChatMessage = (data, options) => { |
There was a problem hiding this comment.
There's a deleted line below that has no matching new line: options = options || {};.
Did you mean to add a default parameter for options here?
| const addChatMessage = (data, options) => { | |
| const addChatMessage = (data, options = {}) => { |
| const $el = $(el); | ||
| // Setup default options | ||
| if (!options) { | ||
| options = {}; |
There was a problem hiding this comment.
I'd suggest moving this to a default function parameter value.
There was a problem hiding this comment.
I don't feel to strongly about this change though, because there's additional "default options logic" that can't be moved into the parameter description, since you may define fade and not prepend, and you still want prepend to default to the value below (following the logic in this code).
| connected = true; | ||
| // Display the welcome message | ||
| var message = "Welcome to Socket.IO Chat – "; | ||
| const message = 'Welcome to Socket.IO Chat – '; |
There was a problem hiding this comment.
(exhibit A of use of single quotes on string with non-parameter)
5 participants
The kind of change this PR does introduce
Current behaviour
The chat example can be updated with ES6 features like default params, constants (avoiding the use of
var), and template literals.https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Default_parameters
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
New behaviour
The chat example with these three features.
Other information (e.g. related issues)