react-famous icon indicating copy to clipboard operation
react-famous copied to clipboard

[mixed-mode] [react-bug] componentWillUnmount calls are in reverse order.

Open trusktr opened this issue 9 years ago • 2 comments

When conditionally rendering a Node, sometimes the Node will be unmounted to be replaced by another one.

When this happens, the Node's componentWillUnmount method is called, followed by it's own children.

When this happens, it's not possible for it's own children to unmount because when Node.famousDelete is called, the _famousNode of the _famousParent is already unmounted!

For example, I've got a class like this:


export default
class SplashScreen extends Node {
    render() {
        return (
            <Node _famousParent={this} id="SplashRoot">
                ...
            </Node>
        )
    }
}

When SplashScreen is used in another component, like this:


// This must contain React components that extend from react-famous/Node
let loginlayouts = {
    SplashScreen, SignUpScreen, LoginScreen
}


class Login extends Node {
    render() {
        return (
            <Node _famousParent  = {this} id="rootNode">

                {React.createElement(this.data.loginLayout)}

            </Node>
        )
    }

    getMeteorData() {
        console.log('login root data changed.')
        return {
            loginLayout: loginlayouts[Session.get('loginlayout')]
        }
    }
}
reactMixin(Login.prototype, ReactMeteorData)

export default Login

then, when the value of this.data.loginLayout changes (and Meteor forces the component to update), the SplashScreen can be unmounted and replaced by another component from the loginlayouts object, and when that happens, I notice the error: SplashScreen's componentWillUnmount method is called, then SplashScreen's rootNode's componentWillUnmount method is called. Since SplashScreen was unmounted first, the call to rootNode's componentWillUnmount fails because there's no _famousParent._famousNode to unmount from.

trusktr avatar Sep 01 '15 03:09 trusktr

Is it possible to change the order of React's calls to componentWillUnmount so that children unmount first? If not, a simple solution may be to add the following check into Node.famousDelete:

  famousDelete() {
    console.log(' --------------- REACT-FAMOUS, this', this)
    debugger
    if (this._famousNode.getParent())
        this._famousNode.getParent().removeChild(this._famousNode);
    this._famousNode = null;
  }

What do you think @pilwon? I'll make a PR for this. I'm also gonna ask about the order of componentWillUnmount calls in the facebook/react tracker.

trusktr avatar Sep 01 '15 03:09 trusktr

I tested the workaround in my app by doing:

Node.prototype.famousDelete = function famousDelete() {
    if (this._famousNode.getParent())
        this._famousNode.getParent().removeChild(this._famousNode);
    this._famousNode = null;
}

before using react-famous/Node, and it works great.

trusktr avatar Sep 01 '15 03:09 trusktr