Skip to content

Route onEnter invokes with next location, but route component renders with old props.location #3299

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

Closed
aaronbeall opened this issue Apr 13, 2016 · 7 comments

Comments

@aaronbeall
Copy link

Version

2.0.0

Description

I'm having an issue React Router props.location being stale after onEnter changes state during a browser back/forward button press, but not otherwise.

I've already posted on StackOverflow here.

In summary (I'm using Chrome, have not tried other browsers yet):

I have <Route component={MyView} onEnter={enter}> and enter() invokes setState() on MyView with state based on the new location.

  • When route is changed using browserHistory.pushState() then enter() is invoked and MyView is rendered with props.location as the expected value. This is correct.
  • When route is changed by browser back/forward button enter() is invoked with expected location then MyView renders with the old props.location, then renders again a second time with the next props.location. This is a problem because MyView renders with mis-matched data: state has been set based on the new location, but the route shows the old location.

This is a problem because I'm trying to use routes to apply page transitions with ReactCSSTransitionGroup as shown in the react-router examples, but since the MyView renders with the new state based on the new position but has the old props.location it fails to apply the transition before re-rendering the view, then when the second render pass happens it transitions out the new state.

Is this a known issue? Am I doing something wrong?

@taion
Copy link
Contributor

taion commented Apr 13, 2016

You can't use setState like that in onEnter. At the point you're calling setState, your view hasn't yet re-rendered with the new routing state, so you're making it re-render with the new state that you've just set, but with the old router state.

There's not much more we can say without more details about what exactly you're trying to do, but you cannot call setState in an onEnter hook without hitting this problem.

@taion taion closed this as completed Apr 13, 2016
@aaronbeall
Copy link
Author

Actually, calling setState from onEnter works well in the first case (after calling pushState) but doesn't work (exactly as you describe) in the second case (using browser navigation). But I get your point, it probably only works by dumb luck in the first case, not as a supported flow. So how should I update state (in my case update a Flux store that triggers a component setState) when routes change? Surely this is not a novel idea? I want to update state based on route change so that state works whether you use browser buttons, click a link, push or pop a location, and also on initial deep-link load.

To elaborate specifically on what I'm trying to do: transition routes based on the nested-animations example. I have a re-usable component RouteViewport that extracts a portion of the current route path, and uses that as a key for a child of a CSSTransitionGroup to apply a page transition.

Something like this (simplified a bit for brevity):

class RouteViewport extends React.Component {
  componentWillReceiveProps(nextProps) {
    let previousPath = this.props.extractSubPath(this.props.location.pathname);
    let nextPath = this.props.extractSubPath(nextProps.location.pathname);
    if (previousPath != nextPath) {
      this.setState({ previousPath, nextPath });
    }
  }
  render() {
    let { previousPath, nextPath } = this.state;
    let transitionName = previousPath.split("/").length > nextPath.split("/").length ? "back" : "forward";
    return (
      <ReactCSSTransitionGroup transitionName={transitionName} >
        <div key={currentPath}>{this.props.children}</div>
      </ReactCSSTransitionGroup>
    );
  }
}

So the general gist here is that when the route changes it extracts a sub-path from the route which that viewport cares about, uses that as the key for the transition group child, and determines whether this route change is "forward" or "backward" for the animation (the actual way that's determined is a bit more complicated, but that part is not relevant and works fine in all cases.)

Now this is an example of how RouteViewport is used:

<RouteViewport extractSubPath={pathForView}>
    <MyView a={this.state.a} b={this.state.b} c={this.state.c} />
</RouteViewport>

State a,b,c passed to MyView is based on the current route: so if I am at /path/to/page/1/2/3 I will end up with data assigned to a,b,c based on ids 1,2,3.

So the thing is, if the route path changes at the same time that state a,b,c changes, all is good. This currently works in my first case (using onEnter to setState from location).

However, if state a,b,c updates before the route changes, you end up seeing the new state rendered in MyView before any transition happens, without any transition, because RouteViewport does not see any path change and trigger the transition group with a new key. Even though a moment later the props.location does change, at this point it's too late and the MyView shows a transition of the new path state exiting then re-entering, instead of the old path state exiting and the new path state entering.

Hopefully that makes sense. I apologize this is such a long description. I'm open to any ideas of how to do this differently.

I should also mention that I can get around this problem by throwing a setTimeout around the setState to ensure that props.location updates first, but I hate timeouts. Is there a more elegant way?

Thanks for your help!

@taion
Copy link
Contributor

taion commented Apr 13, 2016

Why do you need those from onEnter, rather than reading/computing these from componentWillReceiveProps? Are they somehow async?

@aaronbeall
Copy link
Author

The RouteViewport does indeed use componentWillReceiveProps to detect when the route changes.

However the parent route component doesn't check for route change, at this point. If it did also use componentWillReceiveProps would it not need to isolate the portion of the route that might apply to itself? I guess what I can do is expose a callback on RouteViewport like onRouteChange, I just expected onEnter to be the built-in react-router way to do that. Is there a different way? I see router.setRouteLeaveHook but no setRouteEnterHook or setRouteChangeHook?

@aaronbeall
Copy link
Author

Ok, I think I get what you were saying. I added this to my route component:

componentWillReceiveProps(nextProps) {
  if (!equals(this.props.routeParams, nextProps.routeParams)) {
    // update state based on route
  }
}

I guess I was thinking that kind of "when route changes" thing was exactly what onEnter was for. Is there another way to do this from the route component other than manually comparing the route params?

@taion
Copy link
Contributor

taion commented Apr 13, 2016

You want something like params instead of routeParams unless you want specifically the params for that one route, but otherwise that's what I meant.

onEnter is more meant for things like – before loading up the new components, check e.g. that the user is authorized, and redirect to a different path, or else for redirects in general.

I'd use the cWRP hook for now. If this comes up in other contexts, we might consider doing things like #2919 to batch the updates, but I think the cWRP solution is the best for what you're trying to do here.

@aaronbeall
Copy link
Author

Makes sense, thanks for the help.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants