-
Notifications
You must be signed in to change notification settings - Fork 26
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
Forward/Back doesn't trigger navigate could cause restoration headaches #53
Comments
See also #32 where this restriction is discussed more generally. But this particular use case is worth discussing in more detail. The big foundational question you're getting at is, should UI rendering be done in I wonder if maybe the solution is to fire navigate, and allow This could get a bit strange, e.g.:
What would happen here from the JS page's point of view is that:
What would have to happen under the covers in the browser is that we update history entry A's document to be the same as history entry B's document. This kind of modification of past entries is unprecedented, and I wonder if it would cause issues. /cc @natechapin A fallback position would be that we don't allow
|
I think part of this is that we need to differentiate between a new navigation (i.e. pushing a new URL onto the stack) and a directional navigation (i.e. back/go/forward). I think the directional navigations need to be more restrictive in what we allow. You should be able to respond to these events to update the content of the DOM, but as you mention in your example, I don't know that we want people to be able to convert cross-document directional navigations into same-document navigations. That feels reaaaally wild. We have a few cases:
This is seeming to me that maybe we need to bifurcate the navigate event entirely, into navigatations that are adding a new page (where we have lots and lots of controls) vs directional navigations, which are really more about restoration |
Good breakdown. A couple notes:
"Full control" needs the caveat that we can allow updating the content with And indeed, about changing the URL or state, this is making me reconsider the discussion in #5 about using
I lean toward keeping them unified. In particular, consider code such as appHistory.addEventListener("navigate", e => {
if (!e.sameOrigin || e.hashChange) {
return;
}
if (routesTable.has(e.destination.url)) {
const routeHandler = routesTable.get(e.destination.url);
e.respondWith(routeHandler());
}
}); I think this code works mostly fine even for directional navigations. In particular it works fine for SPA app same-document directional navigations. Let's say It fails for cases like my cross-document nav upthread, where the app has decided to let I think it would be fixed by:
Then you would do appHistory.addEventListener("navigate", e => {
if (!e.canRespond || e.hashChange) {
return;
}
if (routesTable.has(e.destination.url)) {
const routeHandler = routesTable.get(e.destination.url);
e.respondWith(routeHandler());
}
}); and the unified event would work perfectly. |
Right, though as always I do wonder whether this might be security theater, as there are other ways to trap users pretty easily without doing anything with responding to the back event (for example, pushing an extra entry in currentchange immediately after a back finishes with the same URL that was removed). We might be better served focusing on giving users control to escape a page altogether rather than trying to limit the JavaScript API. For example, instead of limiting abusive use of alert(), the browser detects and enables the user to prevent the page from showing more. There COULD be instances where blocking the back is actually desirable from a user perspective, so maybe we should let the user decide when they're unhappy with how the page is behaving.
Yeah, I wonder about this too. I kind of lost track where we decided the timing of the URL update as well, as this plays into it.
I do think that people might want to know whether the navigate is a brand new navigate or not. I bring this up knowing it's degenerate, but you could imagine someone reusing the same URL twice in the history stack - and the router could get confused seeing the same URL thinking that they're navigating backwards when in reality they're pushing a new URL that happens to have the same route. There are other differences between new navigates and directional navigates, like directional navigate's destination entry will already have a unique key. Another idea is that we do keep navigate() as is, but we introduce a new event that's earlier ( |
With the current design, you should be able to tell using
Yeah, this might be the right route toward solving #5. |
That's true I suppose but it feels like it's a foundational use case for routers to restore a previous page, vs. navigating to a new page. A lot of routers would store things in memory or elsewhere to make sure that navigations can be snappy. Making it clear how to do that from an API perspective (i.e. a field on the navigate event to indicate if it's a directional navigate or a new entry, but the initiator may help with that), or at least having an example in the explainer would be good. |
I'd love to see an example, ideally small enough to port into the explainer, of how code would behave differently in such scenarios. Would you be able to write one up? |
Roughly...? appHistory.addEventListener('navigate', (event) => {
event.respondWith((async () => {
if (this.currentPage) {
await this.currentPage.transitionOut();
}
if (event.destination.key && this.previousPages.has(event.destination.key)) {
await this.previousPages.get(event.destination.key).transitionIn();
} else {
await this.renderPage(event.destination);
}
})());
}); |
* Closes #53 by allowing respondWith() for same-document back/forward navigations (but not yet allowing preventDefault(); that's #32). * Closes #51 by transitioning from event.sameOrigin to event.canRespond, which has updated semantics. * Does not fire navigate events for document.open(), as per some initial implementation investigations that's more trouble than it's worth. * Adds an example for special back/forward handling in the navigate event, per #53 (comment). * Makes the appendix table a bit more precise about userInitiated, and expands it to cover cancelable and canRespond.
* Closes #53 by allowing respondWith() for same-document back/forward navigations (but not yet allowing preventDefault(); that's #32). * Closes #51 by transitioning from event.sameOrigin to event.canRespond, which has updated semantics. * Does not fire navigate events for document.open(), as per some initial implementation investigations that's more trouble than it's worth. * Adds an example for special back/forward handling in the navigate event, per #53 (comment). * Makes the appendix table a bit more precise in various ways, and expands it to cover cancelable and canRespond.
One primary use case for listening to events is to restore UI.
Imagine for instance, you push a new URL, and in a navigate handler respond to that URL change by rendering a new full page UI and showing it on the page.
Then, say someone navigates back. How are you supposed to update your UI to remove the full page UI you added in response to the last URL update? Should you be listening to
currentchange
? If you do decide to listen tocurrentchange
to handle this use case, how do you differentiate between back/forward navigations and other navigations that are handled by thenavigate
event handler?I think there's a few options to solve this:
navigate
on back/forward but do NOT allow it to be intercepted.navigate
), something likenavigatedirection
.currentchange
so you could know the cases where acurrentchange
is sent but anavigate
is not .There may be other options as well
The text was updated successfully, but these errors were encountered: