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

Is componentDidMount expected to be called twice? #79

Closed
ghost opened this issue Jun 21, 2017 · 36 comments
Closed

Is componentDidMount expected to be called twice? #79

ghost opened this issue Jun 21, 2017 · 36 comments

Comments

@ghost
Copy link

ghost commented Jun 21, 2017

Using version 1.2 in Chrome I can see that componentDidMount is getting called twice.

I am doing an Ajax call in the componentDidMount of my component, so this is not ideal. Is this the expected behaviour?

Example:

 <CSSTransitionGroup className='my-container'
                            transitionEnterTimeout={500}
                            transitionLeave={false}
                            transitionName='example'
                            component='div'
                        >
                        <MyComponent location={location} key={getKey(location.pathname)}>

getKey gets a basic, but unique key from React Router

@jquense
Copy link
Collaborator

jquense commented Jun 21, 2017

Your key is probably changing more often them you think. Every time the key changes react will see the component as different and unmount it and remount it.

@ghost
Copy link
Author

ghost commented Jun 21, 2017

I have logged the key when I click a link to go from the current page (page 1) to the next page (page 2) and I see this:

  • key-page-2
  • key-page-1
  • key-page-2

If I remove the parent <CSSTransitionGroup/> then it just logs key-page-2 as expected.

My transition group will only have direct child item e.g. page 1 or page 2. It might be working as expected, but it seems a bit odd that it is loading the next page, then loading the current page and then finally loading the next page.

@Chryseis
Copy link

meet your same problem, i'm looking forward to how to solve it

@ghost
Copy link
Author

ghost commented Jun 28, 2017

@Chryseis I went with a simple solution:

  1. Removed usage of CSSTransitionGroup
  2. Add an animation class to my element e.g. fade-in
  3. Add an onTransitionEnd event handler to my element which removes the animation class

@Chryseis
Copy link

@rowanwork thanks for your solution

@jquense
Copy link
Collaborator

jquense commented Jun 29, 2017

I'm not sure where your key is logged so its hard to know if anything is weird. If you can put together a reproduction I can take a closer look

@the-simian
Copy link

I'm experiencing this as well. I'll try to add some information - both componentDidMount and componentWillMount of children seem to be firing more than once

I can see the behavior stops when I remove the CSSTransition. In my case I can see that the key is certainly identical, I'm using the location.key and using the transition for transitioning components when the route changes.

I can also see that the double fire only happens on initial page load, not sure why.

@the-simian
Copy link

@rowanwork , Are you still using Transition, or TransitionGroup ? Is the only thing you removed CSSTransition?

@the-simian
Copy link

think my issue might be related to this: #136

@the-simian
Copy link

Ok I worked on this much longer than i'd liked, heres my results. no matter how hard I tried, I could NOT prevent a lot of unusual behavior, like multiple mounting events. At the end of the day I opted for a very brutish, but effective solution - I kept the animation classes in state, and used timers to update them when I needed the classes added and removed.

waaaay simpler, but nontheless got the job done.

@jquense
Copy link
Collaborator

jquense commented Aug 3, 2017

Do you have a repro of the issue? Lifecycle hooks getting called more than once would be a serious bug in React, amoung other things.

@the-simian
Copy link

the-simian commented Aug 3, 2017

I can tell you this happened when I tried to wrap a <Route /> that rendered <Switch /> with a <TransitionGroup /> and <CssTransition />. something about the router and this library don't play nice together unfortunately.

I wish I could dedicate more time to debugging, but I kind of blew my time budget just localizing the issue to the interaction between this library and React Router.

I'm not sure what the best approach is, but for now, the best approach is to do something like this, and completely forego this lib.

class SomeThingLoadedByReactRouter extends Component {
  componentDidMount() {
    let ctx = this;
    setTimeout(function() {
      ctx.setState({
        animationClasses: "slide-enter slide-enter-active"
      });
    });

    setTimeout(function() {
      ctx.setState({
        animationClasses: ""
      });
    }, 2000);
  }

  render() {
    return (
      <div className={this.state.animationClasses}>Your stuff  in here.</div>
    );
  }
}

@browne0
Copy link

browne0 commented Aug 17, 2017

componentDidMount only get's called the first time the view is mounted onto the DOM. So unless you don't set your key as your location.pathname, it can be re-created several times.

There's a exit property that's provided to react-transition-group. Setting it to false would fix your problem, as you're literally just disabling the exit animation another way.

I don't think there's a problem with your component.

Example I used for my portfolio:

<TransitionGroup component="main">
          <CSSTransition
            key={location.pathname}
            timeout={timeout}
            exit={false}
            classNames="fade">
            <Switch location={location}>
              <Route exact path="/" component={Home} />
              <Route path="/blog" component={BlogList} />
              <Route path="/about" component={About} />
              <Route path="/contact" component={Contact} />
              <Route component={NotFound} />
            </Switch>
          </CSSTransition>
</TransitionGroup>

@the-simian
Copy link

I'll swing back another time and try again with the exit={false}, and see how that works

I was using location.key as my key, not location.pathname, but I was able to confirm the key was identical every time.

@nealoke
Copy link

nealoke commented Aug 20, 2017

@the-simian after looking at the example of @browne0 i've found the fix for me. He had the location as a prop on the Switch. After doing this in my case it also causes the componentDidMount to only fire once!

Changing the key to location.pathname or the prop exit to false did not make a difference for me.

Thanks @browne0 for saving my time 🎖

Not working

<TransitionGroup>
	<CSSTransition
		key={location.key}
		classNames="fade"
		timeout={{ enter: 500, exit: 0 }}
		exit={false}
	>
		<Switch>
			<Route exact path={`${basePath}start`} component={Start} />
			<Route exact path={`${basePath}questions`} component={Questions} />
			<Route exact path={`${basePath}comments`} component={Comments} />
			<Route exact path={`${basePath}capture`} component={Capture} />
			<Route exact path={`${basePath}disclaimer`} component={Disclaimer} />
			<Route exact path={`${basePath}finish`} component={Finish} />
		</Switch>
	</CSSTransition>
</TransitionGroup>

Working

<TransitionGroup>
	<CSSTransition
		key={location.key}
		classNames="fade"
		timeout={{ enter: 500, exit: 0 }}
	>
		<Switch location={location}>
			<Route exact path={`${basePath}start`} component={Start} />
			<Route exact path={`${basePath}questions`} component={Questions} />
			<Route exact path={`${basePath}comments`} component={Comments} />
			<Route exact path={`${basePath}capture`} component={Capture} />
			<Route exact path={`${basePath}disclaimer`} component={Disclaimer} />
			<Route exact path={`${basePath}finish`} component={Finish} />
		</Switch>
	</CSSTransition>
</TransitionGroup>

@browne0
Copy link

browne0 commented Aug 20, 2017

@nealoke no problem :)

Unless modified, the location key and pathname are usually the same. If so then the same route will attempt to remount. There is an important reason we pass the location to the Switch component.

Another thing to note, there are two types of locations:
the history.location,
and the location object provided by React Router v4.

A react element rendered by a Switch or Route can provide three props: history, match, and location. The history prop also has a location property, but that's a live object that changes the context every time it changes. The location prop is immutable, which is the reason you pass it into your CSSTransition.

Think of it like a controlled input vs uncontrolled input. A controlled input gets it's value from its parent component. Likewise, a "controlled" route or switch gets the location from the app, while an uncontrolled component gets it from the router object.

Transitions need some type of relationship between different screens in order to know what to do with them. And the best way to let the switch know where you are is to pass it your immutable location.

Hope this helps!

@GuichiZhao
Copy link

GuichiZhao commented Oct 7, 2017

I also came across exactly the same issue and find the solution right here. Even though my problem is already solved, I would like to provide some insight how the solution work.

Firstly, why the componentDidMount invoked twice:
When the key changed, TransitionGroup tend to unmount the component matching the previous key first and then mount the component matching the new key. However, the obsoleted component is not unmounted immediately, rather it keeps in the (virtual)DOM until "exit timeout".
The evil thing happens during the process of unmounting the previously component. Before unmounted, the Switch choose the new Route already.componentDidMount is invoked when the Route match, even though the whole thing is due to unmount.

The solution is when the path change, Prevent Switch choose Route based on latest path, thus, the new component will not be mounted and after timeout, it will be unmounted silently.

By default, Switch use the latest location based on history. By setting location, we force it to choose "previous" one

@the-simian
Copy link

Really appreciate all the information in this thread. If I revisit this I'll try to post my results also. More example code is handy, especially when something is tricky to implement correctly

@fortinmike
Copy link

Thanks everyone, I was having the same issue and this thread allowed me to go back to the important stuff. Thanks to @GuichiZhao for providing details on why this happens, too.

Is this something that could/should be fixed in react-transition-group or react-router, or is there a fundamental incompatibility (without the suggested workaround) between the two?

@ovaldi
Copy link

ovaldi commented May 30, 2018

Meet the same problem.

@silvenon
Copy link
Collaborator

@fortinmike I haven't used React Router in a really long time, so I forgot its syntax, but I'll work on either making react-transition-group play well with it or provide an official workaround.

@huangming1994
Copy link

use react-addons-css-transition-group same here.

@alexdmejias
Copy link

Any thoughts on how to deal with this?

@ghost
Copy link

ghost commented Jul 11, 2018

@alexdmejias My problem was that I was missing the location prop on the <Switch> element.
Have you tried that?

<Switch location={location}> // some routes </Switch>

@alexdmejias
Copy link

alexdmejias commented Jul 11, 2018

This is the POC that I'm working with. Using the latest version of both react-router-dom and react-transition-group.

<BrowserRouter>
    <Route render={({ location }) => (
        <Fill>
            <Navigation />
            <Content>
                <TransitionGroup>
                    <CSSTransition key={location.key} classNames='fade' timeout={300}>
                        <Switch location={location}>
                            <Route exact path='/' render={() => {
                                const A = HOC(testRouteA);
                                return <A />
                            }} />
                            <Route path='/a' render={() => {
                                const A = HOC(testRouteA);
                                return <A />
                            }} />
                            <Route path='/b' render={() => {
                                const B = HOC(testRouteB);
                                return <B />
                            }} />
                        </Switch>
                    </CSSTransition>
                </TransitionGroup>
            </Content>
        </Fill>
    )} />
</BrowserRouter>
const HOC = function (WrappedComponent) {
    return class WithHOC extends Component {
        componentWillUnmount() {
            console.log('++++++++++', 'HOC componentWillUnmount');
        }

        componentWillMount() {
            console.log('++++++++++', 'HOC componentWillMount');
        }

        render() {
            console.log('~~~~~~~~~', 'HOC render', this.state);
            return (
                <div>
                    <WrappedComponent />
                </div>
            );
        }
    };
};

class testRouteA extends Component {
    componentWillUnmount() {
        console.log('++++++++++', 'route A componentWillUnmount');
    }

    componentWillMount() {
        console.log('++++++++++', 'route A componentWillMount');
    }

    render() {
        console.log('~~~~~~~~~', 'route A render', this.state);
        return (
            <div className="test-route">
                <p>test route A</p>
            </div>
        );
    }
}

Component B is the same as component A, just switched the name. Navigating from route A to route B results in the following output:

++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route B componentWillMount
~~~~~~~~~ route B render
++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route A componentWillMount
~~~~~~~~~ route A render
++++++++++ HOC componentWillUnmount
++++++++++ route A componentWillUnmount
++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route B componentWillMount
~~~~~~~~~ route B render
++++++++++ HOC componentWillUnmount
++++++++++ route B componentWillUnmount
++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route A componentWillMount
~~~~~~~~~ route A render
++++++++++ HOC componentWillUnmount
++++++++++ route A componentWillUnmount
++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route A componentWillMount
~~~~~~~~~ route A render
++++++++++ HOC componentWillUnmount
++++++++++ route A componentWillUnmount
++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route B componentWillMount
~~~~~~~~~ route B render
++++++++++ HOC componentWillUnmount
++++++++++ route B componentWillUnmount
++++++++++ HOC componentWillUnmount
++++++++++ route A componentWillUnmount
++++++++++ HOC componentWillMount
~~~~~~~~~ HOC render
++++++++++ route B componentWillMount
~~~~~~~~~ route B render
++++++++++ HOC componentWillUnmount
++++++++++ route B componentWillUnmount

is this what you have @LennardSennep ?

@ghost
Copy link

ghost commented Jul 11, 2018

@alexdmejias Hmm no that is not the issue I had tbh. I had the issue immediately after navigating to an element where you have to navigate 2 times back and forwards? Or am I I reading your console output wrong?

@alexdmejias
Copy link

@LennardSennep in the output above it is just me navigating once from route A to route B, no back and forth

@ghost
Copy link

ghost commented Jul 11, 2018

@alexdmejias Can you try to add the prop exact on the other Routes and see what happens in the console

@alexdmejias
Copy link

alexdmejias commented Jul 11, 2018

exact same output

@ghost
Copy link

ghost commented Jul 13, 2018

@alexdmejias sorry for the late reply. Can you maybe share your repo so I can see what’s going on?

@arcollector
Copy link

arcollector commented Jul 14, 2018

i have the same problem, i solved it by using location.pathname instead of location.key in <Switch> component, i dont know why, but same pathnames outputs differents keys

@silvenon
Copy link
Collaborator

silvenon commented Jul 20, 2018

I added a piece of the documentation accompanied by a demo about usage with React Router. It will be deployed in the next release.

Basically you shouldn't wrap Route or Switch components with a TransitionGroup because it requires hacky solutions like location.key/location.pathname and it's not how this library (in its current form) is meant to be used. You should use CSSTransition at the end of each route, like in the demo.

If you see a limitation in that approach and it doesn't fit into your specific use case, please share it in a new issue, providing a CodeSandbox demo. Thanks!

@Dhirajbhujbal
Copy link

Hi

I have created the one HOC component in React Native
In componentDidMount have written the code for checking Internet Connection

please find the below code

componentDidMount() {
console.log('WrappedComponent ----' + WrappedComponent);
NetInfo.isConnected.addEventListener('connectionChange',this.handleFirstConnectivityChange.bind(this));
}
function handleFirstConnectivityChange(connectionInfo: any) {
alert(connectionInfo);
NetInfo.isConnected.removeEventListener(
'connectionChange',
this.handleFirstConnectivityChange,
);
}

now I am calling this HOC from two tabs ( Tab 1 and Tab 2)
Whenever I am changing the Connection Type my addEventListener connectionChange is triggered two times, Is there any way to avoid this I want to trigger the connection change for once. as I have to load 7 Tabs

Thanks,
Dhiraj

@ghost
Copy link

ghost commented Apr 16, 2019

I think your issue is that you add an Event Listener but not removing it on ComponentWillUnmount(). Always remove Event Listeners if you do not use them anymore or remove it otherwise they will stay in active and could even crash your app in the worst case.

@Dhirajbhujbal

This comment has been minimized.

@justinmacnutt
Copy link

@the-simian after looking at the example of @browne0 i've found the fix for me. He had the location as a prop on the Switch. After doing this in my case it also causes the componentDidMount to only fire once!

Changing the key to location.pathname or the prop exit to false did not make a difference for me.

Thanks @browne0 for saving my time 🎖

Not working

<TransitionGroup>
	<CSSTransition
		key={location.key}
		classNames="fade"
		timeout={{ enter: 500, exit: 0 }}
		exit={false}
	>
		<Switch>
			<Route exact path={`${basePath}start`} component={Start} />
			<Route exact path={`${basePath}questions`} component={Questions} />
			<Route exact path={`${basePath}comments`} component={Comments} />
			<Route exact path={`${basePath}capture`} component={Capture} />
			<Route exact path={`${basePath}disclaimer`} component={Disclaimer} />
			<Route exact path={`${basePath}finish`} component={Finish} />
		</Switch>
	</CSSTransition>
</TransitionGroup>

Working

<TransitionGroup>
	<CSSTransition
		key={location.key}
		classNames="fade"
		timeout={{ enter: 500, exit: 0 }}
	>
		<Switch location={location}>
			<Route exact path={`${basePath}start`} component={Start} />
			<Route exact path={`${basePath}questions`} component={Questions} />
			<Route exact path={`${basePath}comments`} component={Comments} />
			<Route exact path={`${basePath}capture`} component={Capture} />
			<Route exact path={`${basePath}disclaimer`} component={Disclaimer} />
			<Route exact path={`${basePath}finish`} component={Finish} />
		</Switch>
	</CSSTransition>
</TransitionGroup>

I know we have all felt this many times before, but I feel compelled to say, I am not sure that I have ever been so happy to read a git comment more than this one. thanks for taking the time to post.

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