react-router icon indicating copy to clipboard operation
react-router copied to clipboard

<Prompt /> component from react-router-dom misbehaves on cancelling navigation

Open ukhancc opened this issue 8 years ago • 62 comments

I've recently migrated from version 3 to 4.1.2 in the hope that the following bug caused by the setRouteLeaveHook won't be present in the <Prompt /> component from react-router-dom.

So, as per the use case of Prompt component, when the user standing in the form refreshes the page, edit some fields and clicks back button, the confirmation alert pops up and when the user cancels the navigation, it stays on the page but the url gets changed and does not remain the same as of the current form page.

ukhancc avatar Aug 03 '17 07:08 ukhancc

Can you post a more concrete example? I'm not seeing this behavior myself. Are you using a HashRouter, by chance?

timdorr avatar Aug 03 '17 15:08 timdorr

I'm experiencing the exact same thing using BrowserRouter. If the user clicks a link or does an operation that pushes a route to history everything works as expected. If however the user clicks the browser back button I get a prompt as expected but the URL in the address bar has changed to the previous path.

If I then click cancel on the prompt indicating I do not wish to leave, one of two things will happen.

  1. The current route is restored in the address bar
  2. The previous route stays in the address bar

In both cases my current component is still shown and my components lifecycle hooks includingcomponentWillReceiveProps are called which messes up the state of my component.

arvinsingla avatar Aug 03 '17 18:08 arvinsingla

i have the same question. have you resolved?

zhouzefei avatar Aug 04 '17 06:08 zhouzefei

@timdorr just FYI, I'm using the BrowserRouter and my issue is exactly the same as @arvinsingla described "In both cases my current component is still shown and my components lifecycle hooks includingcomponentWillReceiveProps are called which messes up the state of my component."

ukhancc avatar Aug 04 '17 09:08 ukhancc

If any of you are diving into this, PRs are always appreciated for this sort of thing.

At the very least, someone should provide a minimal reproduction repo. It is a lot easier for other people to look into this issue if they have code they can take a look at. Just describing a problem means that anyone who wants to help has to first build their own project before they can attempt to debug, which isn't exactly motivating.

If there actually is an issue with the code here (I'd have to see it happen. I can't think of a reason why what is being described is happening unless there are browser related issues), then it is most likely going to be an issue with history, not react-router.

pshrmn avatar Aug 04 '17 15:08 pshrmn

Also, to make things easier, you can use our codesandbox project as a starting point: https://codesandbox.io/s/n55VljYk7

timdorr avatar Aug 04 '17 15:08 timdorr

CodeSandbox unfortunately won't work here. Their fake browser implementation doesn't work correctly with the <Prompt>. I mentioned it to one of the devs, so they are at least somewhat aware of that issue, but I would consider that bug pretty low priority. That wouldn't be the same issue that is happening here, but it might give the illusion of it.

pshrmn avatar Aug 04 '17 15:08 pshrmn

Ah, I thought they passed that through to the browser. My bad.

timdorr avatar Aug 04 '17 16:08 timdorr

It has some browser integration, but I wish that it didn't. I have some embedded sandboxes in the curi docs website and its annoying that the back button is hijacked. I much prefer Ryan's fake browser that uses an in memory history.

Back to this issue, I was trying to think of why componentWillReceiveProps is being triggered, and there might be an issue there if the navigation was done using the browser's forward/back button. Those are caught by event listeners, and history attempts to reverse the event (counter go(-1) with go(1)). history also will notify any listeners when this happens, but maybe it should be doing nothing when that happens?

pshrmn avatar Aug 04 '17 17:08 pshrmn

I'm running into this this issue as well. I've created a a test case: https://codesandbox.io/s/4lvjoow5mw. I get the same behavior on a real browser.

alexandersoto avatar Dec 04 '17 17:12 alexandersoto

@alexandersoto I loaded your example as a full page (https://4lvjoow5mw.codesandbox.io/) and it worked as expected (although you seem to need to load that page directly, there is some weird behavior if you open it from the editor page).

pshrmn avatar Dec 04 '17 19:12 pshrmn

@pshrmn thanks for checking it out so quickly. The test case appears to work as a full page, but I found another related issue. Click on Broken -> go back -> go back a second time. The dialog box disappears and the url changes to /, but the router is rendering /broken.

Going back a second time cancels the dialog, but doesn't update the url like clicking cancel does. I'm testing this out on Chrome.

alexandersoto avatar Dec 04 '17 20:12 alexandersoto

Hmm, that second case is interesting. The behavior is different between Chrome and Firefox (Firefox will go to the page two pages back).

// given the history
const history = [
  'google.com',
  'sandbox.com',
  'sandbox.com/block'
];
let index = 2;

In Firefox, clicking the back button once will open up the prompt. Clicking it a second time (while the prompt is open) will go to 'google.com'. In Chrome, as you described above, the first click is the same, but the second click will go to 'sandbox.com'. The Firefox behavior is what I would expect to happen.

I also forked your sandbox (https://codesandbox.io/s/l248ml7kj7) to add a third route so I could see what the behavior is when I can go back twice within the same site. When leaving the site entirely, that still works, but in both Chrome and Firefox it has weird behavior because it stacks prompts.

This double prompt issue is actually with the history package, so it would probably be best to work on it over there.

As for the original issue from here, we still need a reproducible test case, so if anyone wants to put one together, it would be appreciated.

pshrmn avatar Dec 04 '17 20:12 pshrmn

I have a same issue with Prompt. Sometimes after clicking browser back button the hash url is returned back, if I say "do not redirect me", sometimes no.. But in both cases the browser is displaying the hash of previous page/state first. My issue is exactly what is described here https://4lvjoow5mw.codesandbox.io/

Stashevich avatar Jan 17 '18 09:01 Stashevich

Same issue here as @arvinsingla described - I'm experiencing the exact same two scenarios, and they both seem to occur quite randomly.

joaquindk avatar Mar 02 '18 16:03 joaquindk

I made a gif of the issue I'm experiencing, which is rather strange behavior: https://gfycat.com/HotNeatInganue

Repro steps:

  1. load the codesandbox example from above (https://4lvjoow5mw.codesandbox.io)
  2. click on the link "Broken"
  3. click back (browser back)
  4. notice how the path in the address bar has already changed to /
  5. click on "cancel" in the prompt
  6. notice how the path in the address bar changes back to /broken

~~The path change between steps 3 and 4 makes React think you've already navigated away before you even get a chance to select "Cancel" or "Ok" on the dialog box, so React unmounts the "Broken" component and then re-renders the "Broken" component after you click on "Cancel" which is when the path switches back. So if you're using <Prompt /> on a page with a form, then you just lost all the data saved in the form component's state due to the unmount.~~ jk, there was another issue that was causing the unmount

zxlin avatar Mar 07 '18 02:03 zxlin

Is there any hack for this to be solve in the mean time?

neilpalima avatar Mar 16 '18 05:03 neilpalima

I experienced the same issue.

When I come from an in app route, the behaviour of Prompt is working as expected, but when I access the page directly with the url of the view containing the Prompt component and I trigger it, the browser url remains modified even if I click on cancel.

I made a repo to reproduce the issue : https://github.com/Edistra/react-router-dom-prompt-issue

I also made a gif of the issue :

ezgif com-video-to-gif

matthieudesprez avatar Mar 19 '18 19:03 matthieudesprez

I seem to be observing similar behavior without the <Prompt> component, but instead while trying to implement something like https://github.com/ReactTraining/react-router/issues/4635#issuecomment-300465164 (a custom styled popup in my app to handle the back button). The URL bar string changes before my custom popup renders, but the page acts as expected (my popup shows and the app doesn't re-route). The difference is it always seems to change & stay changed after using the browser back button, rather than the reported cases here where the URL flips back after the dialog is closed.

ekilah avatar Apr 24 '18 21:04 ekilah

I have the same problem Look this animation. out-2

Below this part of my code

import React, {Component} from 'react';
import { Prompt } from 'react-router-dom';
...
class AgricultorEdit  extends Component {
  ...
  render() {
    return (
      <div>
        <Prompt
            when={!!props.change}
            message={location => 'Você alterou o cadastro e não salvou, tem certeza que deseja sair desta tela?' } />
        ....
      </div>
    )
  }

Piemontez avatar May 04 '18 20:05 Piemontez

Any updates on this issue @timdorr ? I'm experiencing the same problem, using React Router v4, React Router Redux 5-alpha, and using a pretty similar solution to this one to show a custom modal.

Apparently the issue happens when entering the route via POP action (i.e. by directly accessing/reloading), but not if you come via PUSH (navigating from another route in the app).

Could this be related to the history api instead (https://github.com/ReactTraining/history#blocking-transitions)?

Edit: reference to an old issue in history: https://github.com/ReactTraining/history/issues/367

Fabianopb avatar May 14 '18 10:05 Fabianopb

Any update on this issue ? I'm facing the same problem

thomasthiebaud avatar Jul 17 '18 19:07 thomasthiebaud

Any update on this issue ? I'm facing the same problem too

CuiZhengyang avatar Jul 31 '18 08:07 CuiZhengyang

This bug is over a year old and one of the most upvoted issues here. It would be very nice to get an update on the status of this issue or some hack to fix this. Thanks!

tobloef avatar Oct 17 '18 12:10 tobloef

In case this is helpful to anyone, I ended up writing my own version of the Prompt component which doesn't have this bug and also prompts for leaving the page by closing the tab. Written in TypeScript and working with history/createBrowserHistory from [email protected], [email protected] and [email protected].

import * as React from "react"
import {withRouter} from "react-router"

// Explanation: This component is meant to prevent accidental page transitions
// which could cause unsaved work to be lost. When this component is mounted
// and the `when` prop is true, it does two things:
//
// 1. Ask for confirmation before allowing page transitions from back/forward
//    browser navigation and react-router Link navigation. This uses the
//    react-router provided history.
// 2. Ask for confirmation before allowing "leave page" browser navigation
//    (close/refresh). This uses the window's beforeunload event.
//
// The first one is what the react-router Prompt component is supposed to do.
// However, it is buggy. See here:
// https://github.com/ReactTraining/react-router/issues/5405

const DEFAULT_MESSAGE =
  "You have unsaved changes, are you sure you want to leave?"

interface Props {
  when: boolean
  message?: string
  history?: any
}

class NavigationLock extends React.PureComponent<Props> {
  public unblock?: () => void

  public componentDidMount() {
    if (this.props.when) {
      this.startBlocking()
    }
  }

  public componentWillUnmount() {
    this.stopBlocking()
  }

  public componentWillReceiveProps(nextProps: Props) {
    if (nextProps.when && !this.props.when) {
      this.startBlocking()
    } else if (!nextProps.when && this.props.when) {
      this.stopBlocking()
    }
  }

  public render() {
    return null
  }

  private onBeforeUnload = (event: any) => {
    // Prompts the user before closing the page, see:
    // https://developer.mozilla.org/en-US/docs/Web/Events/beforeunload
    event.preventDefault()
    event.returnValue = ""
  }

  private startBlocking = () => {
    if (!this.unblock) {
      const message = this.props.message || DEFAULT_MESSAGE
      this.unblock = this.props.history.block(message)
    }
    window.addEventListener("beforeunload", this.onBeforeUnload)
  }

  private stopBlocking = () => {
    if (this.unblock) {
      this.unblock()
      this.unblock = undefined
    }
    window.removeEventListener("beforeunload", this.onBeforeUnload)
  }
}

export default withRouter(NavigationLock)

davidxmoody avatar Oct 18 '18 07:10 davidxmoody

Thanks for this awesome library. Any update on this from official mentainer.

krvikash35 avatar Nov 28 '18 17:11 krvikash35

@davidxmoody Above code does not work when hitting back button. The route url changes and reverts back on cancel and the whole component is reloaded loosing the state, but the direct browser refresh works.

jombie avatar Dec 13 '18 08:12 jombie

Same problem on react-router-dom "^4.3.1" I am using.. I use Prompt to alert user when leaving the current page wherein data/inputs provided are not yet submitted. Is there any fix this time? Thanks.

nioposiquit avatar Feb 15 '19 05:02 nioposiquit

The same with "react-router-dom": "^5.0.0" and without hashes, and without history. Untitled (1)

Tried to create example via codesandbox - works well. Case #1:

  1. Go to edit page
  2. Change value of the input (local state of component, not Redux)
  3. Click logo - alert displayed and works as expected
  4. Click back button and cancel button - local state become empty (value of input) and URL does not change.

Case #2:

  1. On the Edit Page and click refresh
  2. Edit input
  3. Click logo - works as expected
  4. Click back button and cancel button - URL changed to a previous path but current page/component displayed.

MainApp.js

import React from 'react';
import { Provider } from 'react-redux';

import App from './containers';
import configureStore from './store';

const store = configureStore();

const MainApp = () => (
  <Provider store={store}>
    <App />
  </Provider>
);

export default MainApp;

Routes

import React, { lazy, Suspense } from 'react';
import { Switch, BrowserRouter } from 'react-router-dom';

import { PrivateRoute, PublicRoute } from '../components/routes';

const SignInPage = lazy(() => import('./SignIn'));
const Page404 = lazy(() => import('../components/Error404'));
const Homepage = lazy(() => import('./Homepage'));
const Profile = lazy(() => import('./Profile'));

const App = () => (
  <Suspense fallback={null}>
    <BrowserRouter>
      <Switch>
        <PrivateRoute path="/" exact component={Homepage} />
        <PrivateRoute path="/profile" component={Profile} />
        <PublicRoute path="/login" component={SignInPage} />
        <PublicRoute component={Page404} />
      </Switch>
    </BrowserRouter>
  </Suspense>
);

export default App;

ifier avatar May 02 '19 14:05 ifier

Ohai

I don't know whether this helps anyone out, but I had the same issue: On back button I was prompted but the url indeed changed and sometimes it messed up the page sometimes it did not. (react-router/ -dom version 5.0.0)

After some debugging I realized that I have been using components from 'react-router' and 'react-router-dom' completely mixed, sometimes I import them from one sometimes the other. Now this should not matter much, but apparently in this case it does.

After I have ditched 'react-router' in favor of importing everything from 'react-router-dom' it works as 'expected' (Such as on browser back button it : 1. change the url, but does not unload the component, 2. shows the prompt window, 3. if I choose cancel the url is written back and the component is not unmounted or messed up).

I'm also using import {Router, Route} from 'react-router-dom'; with exported history import {createBrowserHistory} from "history"; export default createBrowserHistory();

marton-levay avatar May 09 '19 15:05 marton-levay