Re-do interrupted and aborted navigations #68
Conversation
Closes #38 by making it explicit that the stop button is meant to trigger the navigate event's AbortSignal. Closes #10 by getting rid of queued-up navigations, their associated event, and their associated callback variants to update() and push(). Instead, navigations during other navigations interrupt the ongoing one. As the new example tries to show, this seems to work pretty well. Closes #13 by canceling all pending navigations when another one starts, of which the case discussed there is one example.
|
I think this might also obviate the need for a callback, though if you feel it's still ergonomic I think that's fine too. |
|
Yep, I removed it for now. If we come up with another use case for it then we can easily add it back. |
|
I'll go ahead and merge this, but if anyone has thoughts or reviews after the fact, feel free to comment! |
| 1. `appHistory.current.finished` stays `false`, and `appHistory.current` never fires the `finish` event. | ||
| 1. `navigateerror` fires on `window.appHistory` with an `"AbortError"` `DOMException` as its `error` property. | ||
| 1. Any loading spinner UI stops. (But potentially restarts, or maybe doesn't stop at all, if the navigation was aborted due to a second navigation starting.) | ||
| 1. If the process was initiated by a call to an `appHistory` API that returns a promise, then that promise gets rejected with the same with an `"AbortError"` `DOMException`. |
|
This is awesome. I was considering opening an issue about the queuing but hadn’t investigated enough of the ongoing convos yet to know if it would be redundant. We discovered in our “bootleg AppHistory” (based mainly on what was here maybe a month ago) that we never seemed to want the original queuing behavior in practice. We ended up dropping it and switching to always-abort-whatever-is-ongoing — even adding "signal" on the NavigateEvent as here. I know it’s anecdotal, but different folks reaching similar API/design conclusions feels really positive. |
Closes #38 by making it explicit that the stop button is meant to trigger the navigate event's AbortSignal.
Closes #10 by getting rid of queued-up navigations, their associated event, and their associated callback variants to update() and push(). Instead, navigations during other navigations interrupt the ongoing one. As the new example tries to show, this seems to work pretty well.
Closes #13 by canceling all pending navigations when another one starts, of which the case discussed there is one example.
This does not yet solve #11; I will comment over there momentarily.
/cc @esprehn; I'd love you to take a look if you have time, since this tries to address some of the issues you found with our current setup. Also @tbondwilkinson since it was him and I who originally brainstormed the queuing system that this largely discards.