Update Histories

[agundermann]

This PR attempts to fix/improve the current Histories in a couple of ways:

1. Store state in sessionStorage based on key with BrowserHistory as well
2. Normalize behavior in terms of creating new history entries (part of Dispatch even if path has not changed & normalize Locations #1031)
3. Fix a bug with HashHistory ignoring hashChange after transitioning to the same path that's currently active

While trying to do all those things, I felt like the History classes needed some refactoring. So History is now responsible for managing keys and state, and performing navigations.

Subclasses now have to do the following:

1. Implement push(path, key): entry and replace(path, key): entry instead of pushState and replaceState. These should only apply the given path (and optionally key) to whatever they're working with, but not call this._notifyChange or set this.location
2. For pop events, instead of setting up this.location and calling this._notifyChange themselves, they now have to call this.handlePop(path, entry)
3. Initial location is now set in setup() by calling super.setup(path, entry), after which this.location can be safely read

The (optional) entry object will be merged into state, allowing subclasses to pass arbitrary information to the user based on the current entry. It's also used to signal History that key (and thus state) is supported. In the future, we could add additional props like that, such as current and length.

Also, I had to change the scrolling test to include timeouts; otherwise, the hashchange event timings would have been off, I think. Not sure why I couldn't reproduce this with the old code (not exactly fun to debug 😄). Anyway, as far as I can tell, the scrolling didn't even work before and the test passed due to native browser scrolling (which is still the case..).

Let me know what you think; I'm also open to applying these changes to the old API if you don't agree with them.

@mjackson @ryanflorence @gaearon

[mjackson]

@taurose This is a most welcome PR, thank you! I'll review today.

[mjackson]

This looks great! Thank you!

I think I'd prefer to omit location.path and just have location.pathname and location.query instead. It is the responsibility of history objects to split off the query string and parse it, then create a new Location object from the result. AFAICT, this would only change the behavior of pushState so that we wouldn't try to do a replace when the user calls it with the same URL they're already at, which they can prevent themselves.

I'm also thinking about the server-side API where we ask people to create their own Location object before passing it to Router.run. They'll probably already have a copy of the parsed pathname and query they can use, instead of asking us to parse it again for them.

serveApp(function (req, res) {
  var location = new Location(req.url, req.query);

  // ...
});

Does that make sense @taurose?

[agundermann]

Yeah, that makes sense now. I was a bit confused by not having location.path ;)

I simply moved history.location.path to history.path.

AFAICT, this would only change the behavior of pushState so that we wouldn't try to do a replace when the user calls it with the same URL they're already at, which they can prevent themselves.

The check also ensures that HashHistory.push isn't called with the current path, which it can't handle (window.location.hash = currentHash is a no-op). It's also the default behavior for browsing static sites. So I think it's better to have all Histories behave like that by default. I'd also add some constructor option to opt out, but I wanted to wait for some feedback first. Let me know if you have any ideas regarding the naming and whether to use a simple boolean or a function.

[mjackson]

This looks great!

One other thing I'd like to add to the histories is the ability to add a listener "before change". This would essentially be faking the functionality of beforeunload for single-page apps. Of course, we'd need to listen to window's beforeunload event for when users actually do navigate away from the page, but we should also trigger these just before we push/replace state programmatically.

I was adding this functionality earlier today before I saw this PR, but I stopped when I saw that you were changing a bunch of things to avoid conflicts. However, I really think we need it so we can use it to give users a chance to prevent transitions before they happen. I want to change all of the "will transition from" hooks to use this instead of using goBack to restore the previous URL when users cancel a transition.

It may be out of scope for this PR, but I just thought I'd mention it while you're in that code. If you feel like adding this on, I'd really appreciate it. Since you wrote most of this code @taurose, you probably have the best idea where it could go. If you do add the functionality here, I can modify <Router> to take advantage of it.

[mjackson]

@taurose Also, if you do your work on a branch in the rackt Github org then I can potentially push to the same branch. That would make it a little easier to collaborate on stuff like this :)

[mjackson]

Also related: #1358

[agundermann]

Moved it to this repo.

See #1376.