-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Session history tracking #843
Conversation
@gaearon The extra argument both PRs add to location changing methods definitely seems to be the right place for all things related to the URL, but not contained within the URL. However, the use case which triggered #828 was getting ephemeral data into and out of a Seems like that extra argument will need a bit of design work if all related-to-URL-but-not-in-URL data have to go through it. |
@taurose This is excellent stuff! Thank you! :D A few questions, just to make sure I understand what's going on here:
Sorry if that's a bit of a brain dump! Excited about the direction this is headed though :)
@insin What do you mean? Sorry, just trying to understand. :) |
That's just one reason. The current implementation doesn't take pop events into account at all. So if you click a link on a freshly loaded page, click browser back and then use
I don't see any point in using this for StaticLocation. The purpose is to keep track/be aware of client-side browser session history. With StaticLocation rendering server-side, I don't think we have any way to access this information, unless it was embedded into the URL.
I think that would make sense. I'll see what I can do, though I don't like the idea of the data being stored and modified in a global module (like History.js currently is). So I'd have to add something like
Well, the PR gives you
I don't think it's possible at all to make this work with anything but HistoryLocation without embedding state into the URL. When the user clicks back or forward, we have no way of telling how many steps he went forward or backward. I don't see how sessionStorage would help there.
First of all I should say that this comment about
To quote MDN:
In other words, POP simply means we've gone to a page we've been to before within our app. Even with pushState API by itself you don't get any information about whether the user pressed forward, back, or how many pages have been traversed, hence this PR. That said, I don't think the current workaround is that bad. The session tracking could make it a bit more robust I guess, by only using POP if the current state contains state which we set before. One scenario where the state would not exist is when changing the URL manually. However, what would we do instead in these cases? I'm not sure if it's safe to assume it was a PUSH.. But, as I said, I don't think we can make this PR work reliably with HashLocation without messing up URLs.. |
@taurose Thank you for your detailed response :) I think I'm starting to understand this better.
I see. Looking over the API of history.js it looks like they were able to leverage I'm also starting to think that the Location interface should change to more closely match the History interface. So every location would have |
@taurose I don't know if it helps, but I've been playing around with a new What do you think of the API in 7b9ad81 ? I was trying to enable some of the stuff you mentioned in #767 while working on it. |
I didn't add state support directly to HashLocation because of the broken URLs; instead, I wanted it be opt-in. Do you intend to make it so that state is always used when using HashLocation? As far as I can tell, HistoryJS manages with one param because they don't have to provide session history information ( So I don't think making Your History.js looks good so far. I don't think If we are to support Locations without state, what do you think about having a As far as Location and Router APIs are concerned I'd see two options:
|
I just noticed that changing the hash (manually or via I'm a bit worried that there could be other cases where popstate state ends up being null, though.. Edit: this breaks |
Please note: this commit is still a work in progress! All history objects subclass History and support 2 main methods: - pushState(state, path) - replaceState(state, path) This API more closely matches the HTML5 history API, with the notable omission of the title argument which is currently ignored in all major browsers. It provides the user with the ability to store state specific to the current invocation of the current URL without storing that data in the URL itself. However, history objects that do not use the HTML5 history API (HashHistory and RefreshHistory) store their state ID in the query string. This should help with #767 and #828. This work was inspired by work done by @taurose in #843 and @insin in #828.
@taurose I just pushed some work related to this in 212b9d5. I'm thinking at this point we will probably benefit from having every location (renamed to "history" in that commit) support state other that what they can keep in the URL, so all of them have Would you please take a look at that commit (it's still a work in progress) and let me know if you think you can build sessions on top of it? Thanks again for all your work on this. I'm not trying to supersede it :) Just trying to understand the problem better for myself and perhaps approach it from a slightly different angle. I'm totally open to any feedback/suggestions. |
I've given up on Your commit looks great thus far. However, as much as I like sticking to the HTML5 history API, I'm still not fully convinced it's the best approach. How would you handle setting state after a transition happened? For example saving some view state after a popstate? Edit: Also, don't you think there's a value in allowing people using HashLocation or RefreshLocation to opt out of query modifications (and have state stored by path, or be ignored)? |
@mjackson Any thoughts on this? I'm thinking in order to support setting state after a popstate, we'd have to extend the API somehow. However, I see no way in making this work with At this point, it feels like we could just have the router manage sessionStorage and state, while the History objects provide keys. For example, we could extend the Location object to provide |
Sure, I can work on it later today or tomorrow if you haven't done it already. |
@taurose I'm doing some work around this today, but I don't want to step on your toes. Your insight is super valuable. Want to pair on it somehow? |
@mjackson No worries; I hadn't gotten around to actually start writing. But I think I would
|
can we close this? |
I'm a little bit swamped right now, but I think we need something like this for remix-run/history#157 and to handle remix-run/history#104 cleanly. |
I meant that in the sense that I haven't had a chance to read through this PR thread - but I will do so when I get a moment. |
(Work in progress)
This PR makes the router more aware of the current session history by managing a counter and an id that's stored inside history.state. Before every push, the counter is incremented, and on every location change, it is reloaded from the active state (or reset). The (random) id is used to distinguish between two "sessions" within the same browser tab.
Since only
HistoryLocation
supports state, I also addedutils/addLocationState.js
which can be used to wrap other locations to attach state to the query (for local development, or environments without url bar). Not sure if it's worth having though.The history state is currently exposed via
state.history
:state.history.current
: index of currently active session entry (1-based)state.history.id
: random string id for current "session"; each id has its own series of entriesstate.history.key
: key for caching based on active entry in browser tab (concatenation of the two above, for convenience)Use cases:
goBack()
check (already implemented), perhaps alsogoForward()
andgo(n)
Seems to be related to #828.