Madison Jackson React Chatlog #114
Conversation
|
I'll dig into the test failure more, what I'm seeing with a quick look is that the test will pass if you run it on its own, but when the whole suite is ran, then state is carrying over between tests in a way that it shouldn't and causing the test to fail because the button is already pressed when the test starts. |
|
Ahh, the bug has to do with how the app state is being updated. I found an explanation from one of our former curriculum folks I'll add in the relevant code of App.js. |
| const onLikeClick = (id) => { | ||
| setChatData(chatData.map(chat => { | ||
| if (chat.id === id){ | ||
| chat.liked = !chat.liked | ||
| if (chat.liked === true){ | ||
| setLikeCount(likeCount += 1) | ||
| }; | ||
| if (chat.liked === false){ | ||
| setLikeCount(likeCount -= 1) | ||
| }; | ||
|
|
||
| return chat; | ||
| } | ||
| return chat; | ||
| })); | ||
| }; |
There was a problem hiding this comment.
The way likeEntry updates the messages data is what's causing one of the tests to fail depending on the order it runs in.
"In react-chatlog, inadvertently mutating the state object leads to inadvertently mutating the module-static messages variable, which has the side effect of persisting “state” across the two test runs.
Our useState data chatData is intended to be read-only, but we're editing the existing entries on line 15. What we want to do is make a copy of the message object we're updating, change the liked value on the copy, then return that new object. Essentially, we can make a shallow copy of objects that are not changing, but we need a deep copy of any objects we're updating. One way we could do that might look like:
const onLikeClick = (id) => {
const newEntries = chatData.map((chat) => {
if (chat.id === id) {
// Use spread operator to copy all of entry's values to a new object,
// then overwrite the attribute liked with the opposite of what entry's liked value was
return {...chat, liked: !chat.liked}
}
// We didn't enter the if-statement, so this entry is not changing.
// Return the existing data
return chat;
});
setChatData(newEntries);
};| if (chat.liked === true){ | ||
| setLikeCount(likeCount += 1) | ||
| }; | ||
| if (chat.liked === false){ | ||
| setLikeCount(likeCount -= 1) | ||
| }; |
There was a problem hiding this comment.
Our state chatData contains info on whether a message is liked or not, which means we could derive our heart count from the data by looping over chatData and counting up how many entries have a liked value of True. This would let us remove the likeCount state that we need to manually keep in sync with the changes to chatData. What changes would we need to make for that to work?
|
|
||
| const [chatData, setChatData] = useState(chatMessages); | ||
| let [likeCount, setLikeCount] = useState(0); | ||
| // let likeCount = 0; |
There was a problem hiding this comment.
Gentle reminder we want to clean up commented out code so it doesn't get into our final codebase.
| /> | ||
| ) | ||
| }) | ||
| return <div className='chat-log'>{getChat}</div> |
There was a problem hiding this comment.
I'm seeing some divs for component wrappers that we could replace with more semantic HTML. I'd consider swapping out the enclosing div here for something like a ul to better represent that this is a list of items being displayed. We could also swap out the div that wraps the ChatEntry components to be a li; together these changes could help increase accessibility of the page overall.
|
|
||
| ChatLog.propTypes = { | ||
| entries: PropTypes.arrayOf( | ||
| PropTypes.shape({ |
There was a problem hiding this comment.
Great use of PropTypes & PropTypes.shape!
I'm not sure why that one test is failing when it does turn to a red heart after the first click...I tried re-working it and seeing if there was any other way to represent that it does change to red or if the component needed to be updated, but it turns True when the heart turns red so I'm a little lost?? But I got it to work!