prevent POP actions to push the url into the history.

[lluistfc]

When using your component with react (v16) + react-router (v4) and react-redux (v5) i've encontered an issue when doing a back from the browser.

If we start on page A and go to page B -> push event.
Once we are on page B we go to page C -> push event.
Once in page C we do a browser back, we end on page B -> pop event. but page B gets pushed onto history.
Back on page B we do a browser back, we end on page C

That behaviour is caused by:

if (pathnameInHistory !== pathnameInStore || searchInHistory !== searchInStore || hashInHistory !== hashInStore) {
  this.inTimeTravelling = true
  // Update history's location to match store's location
  props.history.push({
    pathname: pathnameInStore,
    search: searchInStore,
    hash: hashInStore,
  })
}

on ConnectedRouter.js#L38

By adding another condition to check if history action is "PUSH" i've managed to avoid this behaviour.

if (props.history.action === 'PUSH' && (pathnameInHistory !== pathnameInStore || searchInHistory !== searchInStore || hashInHistory !== hashInStore)) {

[supasate]

Thanks @lluistfc. Last time I looked into this and found some caveats. Sadly, I forget it now. Let me take a closer look into it again.

[lluistfc]

Hello @supasate. Any news on this issue?

[vorlov]

pls check this asap, its a critical issue as the page is rendered twice on browser back action

[quoo]

Any updates on this?

[supasate]

Just revisited this issue.

I'm not sure that this fix is addressing general cases of clicking back button or only during the time traveling.

In the case of clicking the browser back button outside of time traveling, I cannot reproduce this behavior -> "Once in page C we do a browser back, we end on page B -> pop event. but page B gets pushed onto history."

From the codebase, page B will be pushed again only if it's in time traveling, i.e. the location in history and the location in redux store are different. By clicking back outside of time traveling, both locations are synced.

[supasate]

@vorlov the page is rendered twice is a separate issue and this PR doesn't fix that. Let discuss in #141.

[pcs2112]

Critical issue. @lluistfc update fixes navigation issues using the browser's back button.

[reallistic]

Here is a (hopefully) a full explanation of what is going on with the code in question:

When using the redux dev tools to "time travel" (I.e. replay previous versions of your state) the browser history can become out of sync. The original code was designed to remedy this via the following intended logic:

• Developer is on Page B.
• Developer uses redux time travel to revert state to a point when Page A was active.
• ConnectedRouter.js detects the mismatch of router state (changed) and history state (unchanged).
• Page A is pushed onto window stack.

However, the unintended consequence is the following:

• User is on Page B.
• User hits the browser back button to go back to Page A.
• A component mount on Page A fires an action that causes state to change.
• ConnectedRouter.js detects the mismatch of router state (unchanged) and history state (updated to be Page A).
• Page B is pushed onto window stack.
• The LOCATION_CHANGED event fires due to the history listener POP'ing to Page A.
• The router reducer updates the store.
• ConnectedRouter.js detects the mismatch of router state (updated to Page A) and history state (on Page B).
• Page A is pushed onto window stack.

[siddhant91]

@supasate @lluistfc Is there any update on when this will be merged and published.

[lluistfc]

@siddhant91 I'm sorry but we depend on @supasate to approve and accept the PR. I've been using the fork I made with my modifications with no issues. I haven't updated it since the modifications I made so it's really outdaded respect this version, give it a try if you want or fork current version and apply that modification.
Sorry I can't be of more help.

[ymaz]

any updates on that?

[jancama2]

Would be nice to have here some detailed comment to explain why it is important to check the action.

[jancama2]

Could we test it to be sure it wont break the next time?

[pybuche]

Hi ! Any updates on that ?

[JesusADS]

can anyone with permissions fix the .gitignore conflict?

[jwineman]

@jancama2 if I PR this same fix with tests and no .gitignore changes would you accept it?

[roryp2]

Is this going to be updated and merged? This fixes #262

[jarommadsen]

This also fixes #349.

[supasate]

Sorry all for keeping this PR for a long time! TBH, I still don't have time to think about it thoroughly, but, it seems to fix many issues listed above. So, I'm merging it first. If anyone finds issues after merging, please let me know!

[pybuche]

Great, thanks ! Do you know when this will be released?