Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Aug 7, 2020
Merged

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

merged 1 commit into from
Aug 7, 2020

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