[bug/breaking change] CSSTransitionGroup in React 15.1

[mjrussell]

I hope this is the correct place to file this issue, its definitely related to the ReactCSSTransitionGroup but it could be also related to React Router.

Im using React Router with ReactCSSTransitionGroup in order to provide an animation while the user navigates between routes in my app (for instance to provide a nice fancy login animation). I also use a HOC library I wrote (redux-auth-wrapper) to secure my routes with authentication/authorization checks. The HOC performs redirects by using redux state data about user authentication and dispatching redirects (typically via redux actions but can use the React Router context as well).

In 15.0.2 the HOC works perfectly and then upon upgrading to 15.1 there are infinite redirection loops.

Its not clear to me if using the transition group in this way is wrong, obviously a concern is that both the pages are briefly rendered and the HOC (which uses componentWillMount/componentWillReceiveProps to trigger the redirect) is forcing another redirection. But it seems like the cWRP is being called as it is trying to be unmounted and is continually causing cWRP to triggered.

I've created a sample repo at https://github.com/mjrussell/react-css-transition. When you use the master branch, everything works as expected (click to the Foo link and type user name and then should the welcome text) then, moving to the the 15_1-upgrade branch and re-npm install and try to perform the same process will result in an infinite redirect loop (beware of crashing your tab/browser). I typically open a new window, open chrome dev tools and as soon as it loads and starts looping using the pause script button in the console.

I've only tested this in Chrome version 51.0.2704.84

Thanks! Let me know if there's anything else I can do to help

[gaearon]

Can you please try this with the master build of React?

[mjrussell]

Will do, @gaearon is it possible to use npm link with react-addons? Haven't had to depend on a git branch for an addon before

[gaearon]

I usually clone the React repo, run grunt build, and then copy the files from build/modules into the project’s node_modules/react/lib/.

[mjrussell]

@gaearon I tried the master build and had the same result as 15.1 (infinite loop redirects)

[gaearon]

Shouldn’t

  componentWillReceiveProps(nextProps) {
    this.ensureNotLoggedIn(nextProps)
  }

  ensureNotLoggedIn = (props) => {
    const { isAuthenticated, replace, redirect } = props

    if (isAuthenticated) {
      replace(redirect)
    }
  };

be more like

  componentWillReceiveProps(nextProps) {
    const { isAuthenticated, replace, redirect } = nextProps
    const { isAuthenticated: wasAuthenticated } = this.props

    if (!wasAuthenticated && isAuthenticated) {
      replace(redirect)
    }
  }

?

This seems like more correct logic to me: React may call componentWillReceiveProps as many times as it likes. There is no guarantee that it only calls it when something changes, so you should check if the value changed yourself. See also https://facebook.github.io/react/blog/2016/01/08/A-implies-B-does-not-imply-B-implies-A.html.

[mjrussell]

Ah interesting, yeah I would agree that seems like a much safer approach. I typically follow that pattern elsewhere in my code but I think got lulled into thinking that the redirect should happen immediately and checking for change wasn't necessary. I'll give that a try.

[keyz]

By the way there's a typo (trasitionOnLeave) at https://github.com/mjrussell/react-css-transition/blob/master/components/App.js#L28 -- although it's probably unrelated to the redirection loop issue.

[gaearon]

I’m closing because I see no indication of this being a bug in React. #7025 (comment) fixed the infinite loop for me when logging in.

I still have an infinite loop when logging out but this seems to be a problem with redux-auth-wrapper. It assumes that ensureAuth() will synchronously update props but this just isn’t how React works. You can dispatch an action, but there is no guarantee that you won’t receive the same old props another time. In fact it will get even more relaxed in the future with incremental reconciler. So it’s unwise to do some action inside componentWillReceiveProps based on the next props, especially if that action leads to a prop change by itself 😉 . The best you can do there is to detect when a prop changes (by comparing it with the previous value), but please don’t expect that React synchronously updates the props, or that it won’t call componentWillReceiveProps with old props.

[mjrussell]

@gaearon thanks for the input and suggestions, going to work this tonight and improve redux-auth-wrapper with your suggestions.