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

Aborting Prompt after page refresh does not restore correct location. #5383

Closed
trixn86 opened this issue Jul 28, 2017 · 1 comment
Closed

Comments

@trixn86
Copy link

trixn86 commented Jul 28, 2017

Version

4.1.1

Test Case

https://codesandbox.io/s/LXvDvzwX

Note: The sandbox doesn't show the correct behaviour at all. The location never changes back when canceling the prompt. The described behavoir is based on the observed behavior on my local machine.

Fast local setup (<1 min):

create-react-app router-prompt-issue
cd router-prompt-issue
npm install --save react-router react-router-dom

Edit index.js and paste the code:

import React from 'react';
import ReactDOM from 'react-dom';
import {BrowserRouter as Router, Switch, Route, Link, Prompt} from 'react-router-dom';
import registerServiceWorker from './registerServiceWorker';

const List = () => (
    <ul>
        <li><Link to="/1">Item 1</Link></li>
        <li><Link to="/2">Item 2</Link></li>
    </ul>
);

const Detail = ({id}) => (
    <div>
        <h1>{`Item ${id}`}</h1>
        <Prompt when={true} message="Really go back?" />
    </div>
);

const App = () => (
    <Router>
        <Switch>
            <Route path="/" exact component={List} />
            <Route path="/:id" exact render={props => <Detail id={props.match.params.id} />}/>;
        </Switch>
    </Router>
);

ReactDOM.render(<App />, document.getElementById('root'));
registerServiceWorker();
npm run start

Steps to reproduce

  1. Open the app in the browser
  2. Navigate to /1 by clicking on the first link.
  3. Refresh the page.
  4. Press the browser back button.
  5. Press Cancel/Abort in the prompt which is showing up.

Expected Behavior

The location will be restored after "aborting" to show /1 again.

Actual Behavior

The location in the browser url bar stays at / not representing the current location matched by the router.

Notes

This problem only occures when you refresh the page before you press the browser back button. If you do not refresh before clicking the browser back button the location changes back to /1 as expected.

@pshrmn
Copy link
Contributor

pshrmn commented Aug 31, 2017

This is already a known deficiency with history (see this comment). I'm going to close this because it is an issue with history, not react-router, but I'll go into some detail about why this is happening.

  1. When history responds to location changes initiated by the browser, it gets them by listening for popstate events. By the time that the popstate event is emitted, the location change has already happened (window.location is updated and so is the URL in the address bar).
  2. popstate events are very generic. There isn't a great way to differentiate clicking the forward button from clicking the back button to calling window.history.go(239). In theory, there is window.history.length, but that is unreliable. Instead, history stores an array of known visited locations. When it has to revert a pop, it searches through the array to find the index of the current location and of the new location, calculating the difference between them. Then, it calls history.go(delta) (where delta is the difference between them).
  3. When the page is refreshed, this array of visited locations is lost. Before the refresh, the array looks something like ['/', '/1'] (although it actually uses location keys). After the refresh, that array instead looks like ['/1']. As noted in the comment above, when a location is not found in the array, its index defaults to 0. That means that when history attempts to revert the back button click, it uses 0 for the index of both /1 (because it is the 0th item in the array) and / (because it is not in the array). Then, the delta is 0 and history does not call window.history.go.

So what can be done about this? As noted in the above comment, the sessionStorage feature should get rid of this issue. You could maintain the list of known locations in session storage and reload those when creating history (by checking if the array contains the current location's key). This would be a good opportunity for someone who wants to contribute to OSS. To be honest, it isn't the simplest fix, but I wouldn't consider it prohibitively difficult either.

Also, just as an FYI, you don't have to include react-router in your npm install. It is a dependency of react-router-dom, so installing that will install react-router automatically.

@pshrmn pshrmn closed this as completed Aug 31, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 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