Not all issues with back button are resolved

[smashercosmo]

Consider the following routes structure

<Route path="/" component={Main}>
  <IndexRoute component={LandingPage}/>
  <Route component={App}>
    <Route path="product" component={ProductPage}/>      
  </Route>
</Route>

When I go from LandingPage to ProductPage (simply clicking <Link to="/product">) and then try to go back, using browser's back button, I have to press back button five times for this transition to happen.

[davidkpiano]

@jlongster I suspect the problem is here:

    if (routing.path !== locationToString(window.location) && !routing.noRouterUpdate) {
      history.pushState(null, routing.path);
    }

With createHashHistory, this statement: routing.path !== locationToString(window.location) will always be true (as a new hash is always created with an appended string of characters that's never equal to the intended routing path).

In general, the problem is that all actions are triggering a router state change. The actions need to be filtered so that only route-specific actions can trigger a state change.

[jlongster]

@davidkpiano you're looking at an out-of-date source. Look at https://github.com/jlongster/redux-simple-router/blob/master/src/index.js#L68. Only specific route changes will trigger a change.

[jlongster]

@smashercosmo thanks for the report, that's very interesting. I'll see if I can reproduce.

[smashercosmo]

@jlongster Sorry for not providing reproducible example, just hadn't time for that. If you're not going to dig into that problem right now, I can make a demo repo by tomorrow evening.

[jlongster]

I'll try to reproduce for a few minutes but if I can't I'll wait for your test case. Thanks!

[smashercosmo]

deal)

[smashercosmo]

@jlongster any luck in reproducing? Should I make a live example?

[stuartc]

Just bumped into this, I can reliably see the changeID not changing when I hit the back button since the avoidRouterUpdate is true.. FWIW, I got to the next location via updatePath and not a <Link> component.

[leblancexplores]

I have a live example here:
https://github.com/LeBlaaanc/react-webapp/blob/master/src/components/molecules/SignUpLink/SignUpLink.jsx#L19

Once the action fires I'm brought to the signup page but can't go back.

[jlongster]

Thanks a lot everyone. Because of Thanksgiving I haven't been able to look into it yet.

The changeId should not update when pressing back. That only tracks state changes and signifies that we need to call back into the router. The router should be the one that already handles the back button automatically, and rerenders the routing components. All we need to do in that case it update the state in the store.

@stuartc That's a good use case: hitting the back after updatePath. For some reason that may not be working. I'll try to look into it, but I'm traveling all day tomorrow.

@smashercosmo Take a look at @LeBlaaanc and if you think that's a good example then I can take a look there.

[jlongster]

BTW I just merged #38 which was a decent patch, but the sync logic is basically the same so I don't think I solved this. But it might have changed something.

[stuartc]

@jlongster no worries man, on a bit of a tight deadline myself. But I managed to get the back button to work by removing avoidRouterUpdate from the history.listen subscriber.

What is the intent of the avoidRouterUpdate flag? Surely we want to update the router store when the path has changed? I naively can't think of a reason where we wouldn't want that.. I must be missing the baseline behaviour around the listener.. :)

[jlongster]

@stuartc We need a way to break the cycle, because two objects are listening to each other. If we don't break it, they will recursively call into each other.

To be clear, we break the cycle both ways. If the app state changes it'll update the router, but the router will check the current path and compare it with what's in the state and see that it's the same and do nothing. If the router changes, it will fire the updatePath action and update the state, but we need a way in the store listener to not call back into the router. That flag just tells it not to.

The router has already been updated; the event is coming from the router itself after it updated. So we don't need to update it.

It probably still works because it is calling back into the router but the path is the same now so it stops. But I'm surprised you aren't seeing additional history entries; the store listener should be calling pushState again now (although, you may be using the latest version of react-router where calls with the same path actually do a replace, so no additional entries).

I'm going to look into this more now.

[jlongster]

@LeBlaaanc you're problem is that you are using an tag and not preventing default. You are updating the URL twice; the does it natively and then you are calling updatePath, so it makes sense you'd have to press back twice.