Skip to content

Derive chat state from props [#233].#235

Merged
leeschumacher merged 1 commit intojkk:masterfrom
beejunk:issue-233
Aug 7, 2020
Merged

Derive chat state from props [#233].#235
leeschumacher merged 1 commit intojkk:masterfrom
beejunk:issue-233

Conversation

@beejunk
Copy link
Contributor

@beejunk beejunk commented Jul 20, 2020

A section of the code was commented out that was responsible for updating the chatSections state depending on the incoming props. This kept all messages in the chat from displaying, since chatSections is an empty array on initial render and would never get re-computed.

The original code used a method called componentWillReceiveProps that has since be renamed to UNSAFE_componentWillReceiveProps, which is probably the reason it was commented out. This PR uses the static method getDerivedStateFromProps to adjust the chatSections state in a safe manner. It only updates the state if it detects a difference in the number of messages displayed.

This resolves issue #233

@beejunk beejunk changed the title Derive chat state from props. Derive chat state from props [Issue 233]. Jul 20, 2020

_chatScrollRef: ?HTMLElement;

/* componentWillReceiveProps(nextProps: Props) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the original code that updated the chat. Looks like it's been commented out for a couple years now.

@beejunk beejunk changed the title Derive chat state from props [Issue 233]. Derive chat state from props [#233]. Jul 20, 2020
@beejunk
Copy link
Contributor Author

beejunk commented Jul 28, 2020

Vercel deployment: https://shinkgs.ectopod.vercel.app

return { ...state, chatSections: currentChatSections };
}

return null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

return [] in this case to match previous behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returning null in this case indicates that the component state shouldn't be changed from whatever it currently may be. It's a requirement of the getDrivedStateFromProps method: https://reactjs.org/docs/react-component.html#static-getderivedstatefromprops

In other words, it's saying that no new chat messages have come in from the props, so the component state doesn't need to reflect that.

The chatSections state is being defaulted to an empty array on line 65, so it should always at least be that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, thanks!

@leeschumacher leeschumacher merged commit ca33142 into jkk:master Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants