Skip to content
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

TransitionGroup shouldn't try to setState if it has been unmounted #164

Closed
craigglennie opened this issue Aug 16, 2017 · 4 comments
Closed
Labels

Comments

@craigglennie
Copy link
Contributor

I need to use CSSTransition to help with animating a modal on and off the screen. We're using the portal approach, wherein a new React tree is created on a root node (child of document.body), which allows the modal to take over the whole screen. This is required because we have some elements that contain modals, but which create a new stacking context, which breaks (for our purposes) absolute and fixed positioning. This a common approach for modals.

What I want to do is create the portal DOM node when the modal is displayed, and destroy it when the modal is closed. I don't want to have a permanent portal DOM node mounted at the root (we are transitioning to React, and have majority non-React JS; we want React to be self-contained). My approach is to use the onExited handler of CSSTransition to detect when a Modal has finished animating off the screen, and then unmount the whole React portal and remove it's container from the DOM. This works without error, but I am getting a warning from React:

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. Please check the code for the TransitionGroup component.

I've tracked it down to the handleExited function in TransitionGroup. It's calling my handler function, which is causing the whole React tree to be unmounted, and then it calls setState, triggering the warning. I believe this can be easily fixed by checking whether the TransitionGroup has been unmounted, per this doc.

I have created a JSFiddle to demonstrate the issue here. If you have the console open you will see the warning when you click Close Modal (the fiddle doesn't work repeatedly, so you need to refresh to see the warning again).

My proposal is to change TransitionGroup following the example in the doc I linked above.

componentDidMount() {
  // existing stuff
  this._isMounted = true;
}

componentWillUnmount() {
  this._isMounted = false;
}

handleExited = (key, node, originalHandler) => {
  // existing stuff

 // Add new guard before calling setState
 if (this._isMounted) {
    // existing call to this.setState
 }
}

If it's agreed that this is an issue I'll be happy to submit a PR for it.

@jquense
Copy link
Collaborator

jquense commented Sep 29, 2017

happy to take a PR for this :)

@revskill10
Copy link

Any update on it ? I got this warning with material-ui on the server.

@scottdickerson
Copy link

I'm hitting this as well and it's failing my automated tests as the component is unmounted before the final setState callback triggers..

@jquense
Copy link
Collaborator

jquense commented Dec 10, 2018

🎉 This issue has been resolved in version 2.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants