The following React lifecycle methods have been deprecated by React and will start to be removed starting in React 17:
The React team has published a guide discussing the changes and how to migrate to the new lifecycle methods here: https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html
As part of a campaign to remove these methods from https://github.com/mattermost/mattermost-mobile, these methods should be replaced in the following files:
If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.
New contributors please see our Developer's Guide.
can I work on this?
Thank you @sowmiyamuthuraman!
Hey @sowmiyamuthuraman,
How is your work going? Can we help with anything?
yeah! I am working on it. I need some help regarding when to use getDerivedStateFromProps
and componentDidMount
lifecycle methods.
Would you mind posing your question here so we can help?
Hi canni work on this?
@a-c-sreedhar-reddy sure! Thank you :)
@jasonblais There is an eslint rule which does not allow useage of setState in componentDidUpdate. Can I disable that for this component because we had to update the state when the props change.
I think in list component instead of storing the sections in the state better way would be to calculate it in render.
I'm not sure - @hmhealey would you know the answer to the question from @a-c-sreedhar-reddy?
@a-c-sreedhar-reddy Ideally, we'd avoid setState
in componentDidUpdate
since that causes the component to render twice in a row, and we'd either remove the state or use something like deriveStateFromProps
instead, but there will be cases where the extra re-render is acceptable to avoid complicating the rest of the code. I'll let you know in your PR if there's anywhere that I think we could get rid of it. In that case, you can use an eslint-disable-line
on the setState
call since we'd still like to discourage that pattern by default
@hmhealey
I could have removed sections
from state and memoed it in the render method. But I did not because sections
state is used in componentDidUpdate.
I could have used getDerivedStateFromProps
to compute sections state. But then sections
state would be computed even if one unrelated prop changes eg(favoriteChannelIds
).
buildSections
result could not be memoed or it is not possible to know which props have updated since getDerivedStateFromProps
is static.
So I went with componentDidUpdate
.
I also felt it would be easy to refactor this component to a functional component in the future if componentDidUpdate
is used which would be equivalent to useEffect
.
Yeah, that should be fine. I used to be more concerned with avoiding the extra render, but it makes everything much more complicated in cases like this when we're wanting to recalculate a complex value when a certain prop changes. You could theoretically also store the prop in state since that lets you compare with the previous value in getDerivedStateFromProps
, but that's a lot of extra logic which can add other issues.