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

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

Open
trusktr opened this issue Sep 1, 2015 · 2 comments

Comments

@trusktr
Copy link

trusktr commented Sep 1, 2015

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
Copy link
Author

trusktr commented Sep 1, 2015

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
Copy link
Author

trusktr commented Sep 1, 2015

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 trusktr changed the title [mixed-mode] [bug] componentWillUnmount calls are in reverse order. [mixed-mode] [react-bug] componentWillUnmount calls are in reverse order. Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant