React-coding-challenges #1
Conversation
| "socket.io-client": "^3.1.3", | ||
| "use-sound": "^2.0.1", | ||
| "use-sound": "^4.0.1", |
There was a problem hiding this comment.
Upgraded use-sound as the 2.0.1 was buggy (joshwcomeau/use-sound#42) and causes unwanted re-rendering.
Before, every playSend and playReceive causes the component to be re-rendered, which isn't necessary.
| } | ||
|
|
||
| const onChangeMessage = (e) => { | ||
| setCanSend(!!e.target.value); |
There was a problem hiding this comment.
Refactored this Footer component so that when the chat thread updates, this component won't be re-rendered.
|
|
||
| export default function Message({ nextMessage, message, botTyping }) { | ||
| const Message = React.memo(({ message, isLast }) => { |
There was a problem hiding this comment.
Refactored this Message component to avoid unnecessary re-rendering when botTyping changes.
| import { MY_USER_ID } from '../../UserList/constants/users'; | ||
|
|
||
| const BOTTY_USER_ID = 'bot'; | ||
|
|
There was a problem hiding this comment.
Refactored and moved the message list (chat thread) component into a separate component, so as to separate the Socket operations from message list rendering. Otherwise, the Socket event listeners will be frequently registered/removed on message list changes (new message added or botTyping changes)
| const isLast = (!nextMessage && (!botTyping || message.user === MY_USER_ID)) | ||
| || (nextMessage && nextMessage.user !== message.user); | ||
| return !!isLast; | ||
| } |
There was a problem hiding this comment.
Moved the isLastMessage check out of the Message component to avoid the Message component from being re-rendered whenever botTyping changes.
| }) | ||
|
|
||
| return () => sub$.unsubscribe(); | ||
| }, []); |
There was a problem hiding this comment.
Above change can make sure only the user Bot's latest message gets re-rendered. Before, all users got re-rendered whenever new bot message came in.
|
|
||
| // Publish global latest messages change | ||
| subject.next({ ...messages, [userId]: value }); | ||
| }, []); |
There was a problem hiding this comment.
Using useRef & Observable instead of previous useState can avoid unnecessary child component re-rendering. Furthermore, avoid unnecessary Socket listener registers/deletions.
|
|
||
| const BASE_URL = `https://api.spotify.com/v1/browse`; | ||
| const CLIENT_ID = process.env.REACT_APP_SPOTIFY_CLIENT_ID; | ||
| const CLIENT_SECRET = process.env.REACT_APP_SPOTIFY_CLIENT_SECRET; |
There was a problem hiding this comment.
Saving CLIENT_SECRET in frontend is unsafe and should be moved to backend. However, I just keep it in frontend as the README guided. And considering this is an interview coding challenge, I skipped implementing a "real" backend authentication solution.
| 'Content-Type': 'application/x-www-form-urlencoded', | ||
| } | ||
| return httpPost$('https://accounts.spotify.com/api/token', body, headers); | ||
| } |
There was a problem hiding this comment.
Get the auth token based on Spotify API doc https://developer.spotify.com/documentation/web-api/tutorials/client-credentials-flow.
| finalize(() => authTokenSource$ = null), | ||
| share() | ||
| ) | ||
| } |
There was a problem hiding this comment.
Above is a singleton design that can make sure concurrent Spotify requests can maximumly trigger only one request to get the access token.
| React.useEffect(() => { | ||
| const sub$ = getCategories$().subscribe(setCategories); | ||
| return () => sub$.unsubscribe(); | ||
| }, []) |
There was a problem hiding this comment.
Trigger three APIs in three useEffect calls so that they won't interfere with each other. And they will be triggered as quickly upon the React runtime and browser's resource (like concurrent request limit per host). In reality, they might be in parallel or not.
| 'messages__message--last': (!nextMessage && (!botTyping || message.user === ME)) | ||
| || (nextMessage && nextMessage.user !== message.user) | ||
| 'messages__message--me': message.user === MY_USER_ID, | ||
| 'messages__message--last': isLast |
There was a problem hiding this comment.
Before, passing botTyping in will cause all Messages being re-rendered when bot is typing, which isn't necessary. After change, only the last one Message is re-rendered.
Finished two hard level challenges.
Chatter (Hard) ✅
I mainly did below changes.
use-soundFooterMessageMessages(separate chat thread part out toMessagePanelList)UserandUserListLatestMessagesContextto useuseRefandObservableinstead ofuseStateto avoid unnecessary descendant components from being re-rendered when state changes.Spotify (Hard) ✅
I mainly did below changes.
REACT_APP_SPOTIFY_CLIENT_IDandREACT_APP_SPOTIFY_CLIENT_SECRETfrom frontend env vars, according to Spotify API doc and.env.local(git-ignored). This is an unsafe way, but adding in frontend is just for completing the challenge. In production design, they shouldn't be disclosed in frontend and should be handled by backend.rxjsinstead of nativePromisefor showing off purpose. Specifically for this challenge,rxjsseems overkilling.Promise.alland sequencialawaits can work as well.Notes: