UIP-2186 Don't call setState when AbstractTransitionComponent is unmounting #59
Conversation
This resolves a warning of the form "Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op."
RavenNumber of Findings: 0 |
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
==========================================
+ Coverage 97.72% 97.72% +0.01%
==========================================
Files 28 28
Lines 1356 1358 +2
==========================================
+ Hits 1325 1327 +2
Misses 31 31Continue to review full report at Codecov.
|
+10 Thanks for the contribution! |
|
+1 |
|
|
||
| @override | ||
| void componentDidMount() { | ||
| _isMounted = true; |
There was a problem hiding this comment.
Doesn't this fall back to the antipattern of tracking mounts?
https://facebook.github.io/react/blog/2015/12/16/ismounted-antipattern.html
There was a problem hiding this comment.
No that is the isMounted method on a React component. They actually recommend this pattern:
"An easy migration strategy for anyone upgrading their code to avoid isMounted() is to track the mounted status yourself. Just set a _isMounted property to true in componentDidMount and set it to false in componentWillUnmount, and use this variable to check your component's status."
There was a problem hiding this comment.
Fair enough. Just saw it and noticed.
|
QA +1
Merging. |
This resolves a warning of the form "Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op."
Ultimate problem:
An AbstractTransitionComponent's
setStatecall to set the transition phase toTransitionPhase.HIDDENcan occur after the component has been unmounted. This may happen if the component contains a button that causes the AbstractTransitionComponent to be unmounted when clicked.How it was fixed:
Added
_isMountedboolean to the component to keep track of whether or not the component is mounted (it's set to true incomponentDidMountand set to false incomponentWillUnmount). The value of this boolean is checked before callingsetStateto set the transition phase toTransitionPhase.HIDDEN.Testing suggestions:
Verify that tests pass.
Potential areas of regression:
Components based on AbstractTransitionComponent.