search

faceyspacey / redux-first-router

🎖 seamless redux-first routing -- just dispatch actions
MIT License
1.57k stars 143 forks source link

url changes before confirmLeave is satisfied #321

Open villesau opened 5 years ago

villesau commented 5 years ago

confirmLeave is a nice feature!

Anyhow, when moving away from that route, url is changed before confirmLeave is accepted. That should not happen. Apparently confirmLeave is called after url has already been changed when it should block url change altogether until accepted.

cdoublev commented 5 years ago

I don't have this issue with confirmLeave: () => 'Are u sure?'. The route action is blocked and the url stays the same. Are you returning a truthy value from confirmLeave?

villesau commented 5 years ago

By returning truthy value, everything works almost like expected. With falsy value url is changed. The result is that url has changed but redux is not updated.

To emphasize, the url is changed before the popup appears, not when returning value from popup.

hedgepigdaniel commented 5 years ago

confirmLeave needs to be a synchronous function - is that the case here?

Here is where the handler is created to check confirmation if you enter a route where you have configured confirmLeave (in _afterRouteChange):

https://github.com/faceyspacey/redux-first-router/blob/0a3ff66832849a634531802ca7e1800cb91ad789/src/connectRoutes.js#L420-L429

Here is where it blocks the navigation in _beforeRouteChange if the handler has been set up for the current route (before the route change):

https://github.com/faceyspacey/redux-first-router/blob/0a3ff66832849a634531802ca7e1800cb91ad789/src/connectRoutes.js#L321-L330

If _beforeRouteChange returns true, then the middleware should skip dispatching the action and changing the URL:

https://github.com/faceyspacey/redux-first-router/blob/0a3ff66832849a634531802ca7e1800cb91ad789/src/connectRoutes.js#L298-L304

Perhaps you could work out which part is going wrong?

hedgepigdaniel commented 5 years ago

I ran into this myself - it happens when you return false from confirmLeave. When reaching this line:

https://github.com/faceyspacey/rudy-history/blob/ce8bed5d97d8c8c423b09a7c3468e237496aea4b/modules/createTransitionManager.js#L39-L40

The redux action is skipped after callback(false) happens (callback(true) seems to be the expected behaviour to allow the transition to occur normally)

You can work around the issue by returning a falsy value other than false (e.g. undefined) from confirmLeave, although I'd say its definitely a bug. Not sure what the purpose of that line is though.

ScriptedAlchemy commented 5 years ago

I’ll ask James. Will one of you place this into the replace the documentation. It’s likely to show up again as an issue haha.

I’m on my mobile for a few days. But will do it if I don’t see a PR ❤️❤️ Well done on the debugging efforts

ScriptedAlchemy commented 5 years ago

@hedgepigdaniel would you mind opening a small or about this?