AlexGalays/abyssa-js

enterPrereqs/exitPrereqs are fundamentally broken

Closed this issue · 1 comments

Short story

For the next major version of Abyssa, I'm considering getting rid of exitPrereqs completely and fix enterPrereqs by always have the router change the url synchronously.

Long story

  • Introduction

Currently Abyssa waits for the whole transition to complete before updating the url. Transitions without enterPrereqs/exitPrereqs will be almost synchronous (They're still wrapped in a promise that will only take one frame to resolve). On the other hand, transitions with enterPrereqs/exitPrereqs can take any amount of time.

  • enterPrereqs
    This functionality works fine when navigating normally: I click somewhere, a transition is initiated, after say, 50ms or 5s the transition finishes, the url then updates. Should the transition fail, the url is not changed. That's great.
    Where things get ugly is when a popState event is involved (back/forward buttons, history.back(), etc).
    When this event is dispatched, the browser immediately updates the url; This default behavior cannot be prevented. This implies that to be consistent with the regular navigation scenario, Abyssa have to revert the url to what it was, wait to see if the transition succeed or fail, then change the url back if it did succeed. This is not only a bit ugly, it is impossible to implement:
    • (Not currently implemented) If we were leveraging the url hash, changing window.location.hash generates a new history entry. We're pushing new history entries in the users' browsers just to block navigation, that's awful.
    • Using the history API, we could call history.replace() to mutate the current state: Same issue: We're transforming the navigation history and losing information; An user could keep on clicking the back button and end up overwriting his entire history with the same, current state. Again, awful.
    • This leaves only one option: Not mutating the history, but navigate it. Note that the browsers don't let one read the full history entries for cross site privacy reasons. We could try to find out where the previous state was in the history (By maintaining our own history stack, and give our states unique ids) and call history.previous() or history.back() a number of times or even call history.go(offset). Indeed, on desktop browsers, one can jump in the history a few entries at a time (e.g hold the mouse on the back button, see the list of the last 15 entries and pick one) Again, this is impossible to implement. The history can be interleaved with entries not belonging to our app so we cannot maintain our own history stack. This means we would have to navigate the history in both direction, trying to find and reactivate our previous state; Considering each navigation operation is an asynchronous operation, this would be a truely insane thing to attempt.
  • exitPrereqs
    This functionality was mostly added for symmetry with enterPrereqs. The problem is exactly the same as for enterPrereqs except I believe there is no rescue plan for it. Since the browser history navigation cannot be blocked, we have to let the url change itself synchronously with the popState event. Now imagine an exitPrereqs asking the user if she really wants to leave this state because of unsaved changes. She hits the "backward" button, see a warning popup, the browser already changed the url to the previous state. She hits "cancel" to remain on the current state: State and url are now out of sync, even though no error occured.

So why is exitPrereqs so much worse than enterPrereqs? It can leave the router's state and the current URL out of sync during normal operations. Here's an example:

The user comes from state A and is currently in state B
The user makes some changes in state B
The user hits her back button to go back to A, either on purpose or not
A warning dialog is plugged into exitPrereqs and appears to try and prevent any work loss
The user decides to stay on the current state B and finish her work so hit cancel => the exitPrereqs promise is rejected
The router is now out of sync with the URL: The state is B with the URL of state A

  • Conclusion

A router that cannot maintain its current state and the url in sync seems a bit stupid to me considering it was its only job in the first place! That's why I'd rather not have features pretending the navigation can be blocked, since the browsers don't allow it.

For information, Ember.js is often said to have the best router of all the full-stack js frameworks; It offers a functionality similar to enterPrereqs (http://emberjs.com/guides/routing/asynchronous-routing/) and has all the issues described above: The url sometimes changes before a transition, sometimes after, and it can easily get out of sync.

So what should be done for Abyssa?

One possibility is to leave things mostly as they are and document enterPrereqs and exitPrereqs as being unsafe under certain conditions (mostly when using urlSync: true). I'm not fond of this option or having "sometimes working functionalities" in general, as it's confusing and if one is using Abyssa, it probably means urlSync is enabled for some devices (Desktops at least), where as we've seen, enterPrereqs / exitPrereqs is broken.

What I suggest instead is getting rid of exitPrereqs. It had few uses in the first place, though in an ideal world, it would be a great feature to have around.
As for enterPrereqs, I suggest "fixing" it by having the url update immediately upon state changes; this way we have the exact same behavior when navigating normally or when popping a state. There will be an inconsistency window while the transition is running. we have guaranteed eventual consistency if the transition succeeds. Should the transition fails, the developer can take action with the router.transition.failed signal, display a message, etc. This respects the browsers' choice of never having blocking history changes except in a few strategic places (window.onbeforeunload). This would also simplify the router's logic.

Any thoughts? Did I miss anything?

The changes discussed above were implemented for Abyssa 5.0.0