-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Move state update listeners from constructor to componentDidMount #28341
Conversation
I'm not really familiar with React's strict mode, so could use some help understanding the changes here. I skimmed https://react.dev/reference/react/StrictMode, but it wasn't that helpful and I could do with some help understanding its specific effects on our application. For example, why do we need to add The PR title is "Move state update listeners from constructor to componentDidMount", but (a) the PR doesn't explain why that is a good thing to do; (b) it's not obvious that A few words in the description of the PR might help understand what's going on here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks sane in general but some clarification would be appreciated
Indeed, without it your inline definition (
I did but thought it much higher risk when in the medium term everything will be getting replaced with functional components which can deal with that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Signed-off-by: Michael Telatynski <[email protected]>
Signed-off-by: Michael Telatynski <[email protected]>
c016f30
to
4550068
Compare
Signed-off-by: Michael Telatynski <[email protected]>
Fixes #28306
Also ensures
this.unmounted
is set appropriately to account for the React 18 strict mode double-render. Without it your inline definition (private unmounted = false;
) might only be called once, and you may be unmounted before you are rendered, so none of your data updates end up applying due tothis.unmounted = true
in the unmount before the remount.