Another fix for #4876, TransitionGroup errors #6710
Conversation
If the component has unmounted before a transition completes, sometimes the ref is undefined therefore it will throw. By adding a check to make sure we actually successfully got the ref, we avoid the error.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
|
CLA signed. |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
I wonder if this is a duplicate of #6538, which has already been fixed in master using a different solution. Can you create a unit test that fails without this change and passes with this change? We should have that as a regression test anyway, assuming this bug is not already fixed in master. |
|
I don't think it's related as this is specifically for ReactTransitionGroup, but I can create a test case as it's pretty easy to repro. |
|
Perhaps this is a fix for #5296 |
This would be most helpful! |
|
I'm a bit busy this week but the info from #5296 should help me write the test case, hopefully this weekend. |
|
I’m going to close this for inactivity.
Thanks! |
|
To clarify my points, since you say
we would need to see a case where this happens and fix the transition child that invokes the callback despite being unmounted. We should be canceling that callback but if we don’t (in this case?), we should fix it there instead. |
|
@gaearon From the docs for
So I think that's why the ref still exists... because the component can unmount without the callback being called. I did not yet have time to write a test to back this up. But, I think the answer is to save the callback to the component instance. i.e. componentWillLeave(callback){
this.leaveCallback = callback;
doSomeGSAPAnimation(callback);
}Then in In that case, no modification to React is needed, but perhaps better documentation that instructs the developer to explicitly clean up custom callbacks... or maybe we catch the error and notify the user of the proper way to cleanup |
|
This sounds correct to me. Perhaps this could be addressed by docs, PR is welcome! |
If the component has unmounted before a transition completes, sometimes the ref is undefined therefore it will throw. By adding a check to make sure we actually successfully got the ref, we avoid the error.
This is a fix for 4876