-
Notifications
You must be signed in to change notification settings - Fork 961
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
Remove history.block #690
Comments
There is also an issue with |
Vue and Angular can provide "block navigation" functionality, so it's doable. This project has 3M weekly downloads, and there are no alternatives for React. I am not able to implement your suggested solution in my current project because it will add too much complexity, and will require rewriting thousands of lines of code. Probably, the client won't like I really like React, but the whole ecosystem is painful. |
Hi, we are happily abusing |
... we are even wrapping our little library around it : / https://github.com/zambezi/caballo-vivo which we use internally on a few very large projects in banking. |
@mjackson - I didn't see the code (yet), but I was thinking - if you remove it, would it be possible to have a way to hook into |
@BetterCallSKY I know for a fact that both the Vue and Angular routers are heavily based on React Router (Vue's history lib in particular is almost a direct copy of this library), so I would not be surprised if their implementations of this feature are subject to the same limitations we are. Instead of just saying "Vue and Angular can do it" and lamenting, you should actually go and build a feature of your app in either of those libraries, post it here, and I'll show you how it has the same limitations I've described above. @cristiano-belloni Yes, it shouldn't be too hard to build out this functionality on your own. Something like this could work: history.listen(location => {
if (!window.confirm('Are you sure you want to leave?')) {
// This is the hard part. You can naively say "go back", but you can't be 100% sure
// that will take them to the location they came from. It's just a guess.
history.goBack();
}
}); |
https://github.com/vuejs/vue-router/tree/dev/src/history https://codesandbox.io/s/jzr5nojn39 |
@BetterCallSKY Look closer, my friend. There are numerous instances where the code in that library is copied straight from v3. One example: here's us and here's them ... even the comments were copied from us 😅 It's been a few years, so the code has changed over time. But when that library started it was a pretty direct copy of this one.
I made a little video to show how that example is broken: Here are the steps to reproduce:
This is because when Vue's router cancelled the navigation from page 2 back to the home page, it could not tell where the user was coming from, so it did the naive thing and pushed page 2 back onto the stack. That's the limitation that I was trying to describe in the OP. |
Thanks for explaining. |
@BetterCallSKY It's easier, safer, and less code to save the form into sessionStorage. |
@BetterCallSKY I'm not saying we can't have a naive |
@cristiano-belloni I was thinking a bit more about your question, and I think this is probably the best generic solution I can come up with: // Use this variable to keep track of the most recent location.
let prevLocation = null;
// Be sure to call this on the initial page load so that you get
// the initial POP.
history.listen((location, action) => {
if (prevLocation) {
if (!window.confirm('Are you sure you want to leave?')) {
// They want to stay. Try to revert the last location change.
if (action === 'PUSH') {
// If it was a PUSH, goBack() should get us back to the last URL we were at.
history.goBack();
} else if (action === 'REPLACE') {
// If it was a REPLACE, we can probably just do another replace with
// the prevLocation.
history.replace(prevLocation);
} else { // action = POP
// This is the hard part. We can only guess what to do here because we
// cannot know where they came from. Should we goBack()? Should we
// push(prevLocation) like Vue does??
}
return;
}
}
// Save this reference for next time.
prevLocation = location;
}); |
Thanks, @mjackson. Will try this as soon as possible! |
Applications can have various complexity. It's definitely not right that you create a low-level library and you force people on high-level UX. |
@BetterCallSKY Please stop. I would like to limit discussion here to helping people instead of arguing about what's "right" and what's not. |
So... I just wanted to hop in here and say: I hate to see folks give up like this. history.block is working perfectly for us, even on iOS. But I do understand being beaten up by edge cases until one finally just gives up. I do have a question, though. My initial reaction is to just wire my own hook into the transition manager to replace the method you're about to rip out... but it suddenly becomes unclear to me when you started boasting of significantly smaller code, is your intention to rip out the entire transition manager? I did try your history.listen suggestion above in our project, but it breaks because we are using (not my choice) a yucky where we
in order to force a component refresh (it's an IFrame component). Certainly not trying to defend this decision here, but when history.listen fires on the .push, the code calls the .back method, but then the location.pathname isn't actually switched back before that if condition occurs on the next line, so according to the code, it looks like the user approved the switch even though they didn't. Without an explicit history.block method, I really do need to hook into the transition manager directly so I can set things up myself, and then things would update inline, which is what we need. So I guess I'm hoping the plan is keep it (the transition manager)? :) |
So the way I'm reading it, it looks like you would need to expose the transitionManager here before people could start implementing their own replacement for block: const history = { return history; |
It’s absolutely doable. I’ve actually forked the lib and implemented it myself the right way. First of all, you need to add functionality that enables you to toggle dispatching events to the listeners on/off. Once you’re in control of that, you can simply replace the url to the previous URL when a user tries to navigate away without notifying the listeners as if nothing has changed. |
What if we would keep the Then for a given history state, we would know:
This would allow me to have a Maybe that would also make the |
@mjackson I also tried the suggestion you proposed in #690 (comment), but as @TheFrogPrince said it wasn't equivalent:
Like @TheFrogPrince , I would be okay with the removal as long as there is a way to build the feature with a public API. |
@TheFrogPrince The "transition manager" is not even public API. It's purely an implementation detail. Whether it stays or goes is irrelevant.
@l0gicgate If you do this your app will be broken in the same way I demonstrated in #690 (comment) After thinking about this a bit more I think there may be a way we can keep |
Thanks for the follow-up.
Looking forward to seeing it ! |
I'd like to remove
history.block
. This is a major, breaking change (we're removing the functionality altogether), and I'd like to explain our rationale.The use case
history.block
was originally introduced to support React Router's<Prompt>
component. The purpose of a<Prompt>
is to prevent a user from navigating away from a page when they have entered some data that hasn't yet been saved, e.g. a half-filled-out form.history.block
provides a way for a<Prompt>
to prevent thelocation
from changing (and thus prevent the router from showing a new page).All other uses of
history.block
are abusing this API for some reason other than for what it was intended.The problem
The main reason I'd like to remove this API is because it is incomplete, and we can never make it complete, so we need to find another way to satisfy the use case. If you've ever browsed the source, you have have noticed some significant
TODO
s around the code that tries to restore the last location when navigation is denied.This is because there is no way we can actually prevent the user from changing the URL, so we have to try and fake it. How?
history.block
callbacksfalse
), try to revert the URL changeNow, we say try to revert the URL change because the simple fact is there is no API in the browser that tells you the last URL you were at. This is a security measure, obviously. Browsers don't want to let you run code that tells you where users came from (with the exception of
document.referrer
, but I digress).But I tried to hack it (I know, I'm a terrible person. This is my penance).
What I did was I keep an array of all locations that the user has visited in memory. Then, when navigation is cancelled I use the index of the last
location
's key to try and figure out how many entries we need togo
in order to restore the last URL. Needless to say, this has a number of drawbacks, not the least of which is that it won't work after a page refresh. So this is a dead end.I originally shipped it thinking it'd be good enough for most apps, and it has worked ok for some apps. But it has also created a lot of headache for people who have run into the edge cases.
The solution
It's not difficult to figure out your own strategy for a good user experience in this situation. It will probably boil down to something like this:
localStorage
(orsessionStorage
)localStorage
so you know it has been submitted successfullybeforeunload
event to prompt them to stay and submit the form first orIt's not hard to imagine wanting to handle some unsaved state in several different ways.
The upsides
The downsides
history.block
because I can't solve this problem for you in a reliable wayI realize this is going to make some people upset (breaking changes always do) but I've tried my best to be open and explain the rationale behind this decision while still providing a way forward. Happy to hear your feedback and questions if you have them.
The text was updated successfully, but these errors were encountered: