Better warning message for external setState called as side-effect of constructor
#5343
Conversation
render method is currently executing. That isn't necessarily true, as the current owner is also set while a component's constructor is executing. This caused warning messages to be printed by mistake when a component's constructor triggers anoterh component's `setState` call. This PR attempts to fix this by adding a new module ReactCurrentRender that keeps track of the component whose `render` method is currently executing. Fixes issue facebook#5313
|
@acdlite updated the pull request. |
|
Wrapped updates to ReactCurrentRender in dev environment checks per @sebmarkbage's comment: #5313 (comment) |
|
@acdlite updated the pull request. |
acdlite
changed the title
setState called as side-effect of constructor
as a side-effect of a component constructor. An example is when a Flux action creator is called from a constructor. These calls should be moved to `componentWillMount` instead.
|
@acdlite updated the pull request. |
|
@jimfb Warning message is now printed, with more helpful/accurate explanation:
|
There was a problem hiding this comment.
We don't need another thing to hold the identity of a component. We explicitly want to get rid of it completely. It would be better if this was just true/false if something is currently rendering since that should be all we need for warnings. This is important step forward because this file can live in the renderer package.
ReactCurrentOwner needs to live in some shared global package with a fixed version number so that it can be shared with ReactElement.
For now, we can use owner for the error message itself but we shouldn't track the instance here.
I understand that you need the instance to disambiguate.
|
A constructor is the same as a render method from certain types of components. E.g. stateless functional components. That's why the owner is set up there. I don't think this solution is accounting for that. Would it be enough to simply rephrase this error message as |
|
Oh, I see... After reading your comment, I had thought that before calling the component constructor, we could check to see if the component was a stateless function, and if so, set a flag that the component is rendering. But I see how that's not currently possible because we don't know if a component is stateless until after you've called it. I believe I understand now why the current implementation exists. In that case I think you're right. It should be sufficient to update the error message. I will close this PR and open a separate one. |
within either `render` or a component constructor. Follow up to facebook#5343
4 participants
Attempts to fix #5313