Skip to content
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

would it be possible for createBrowserHistory to also have entries and index? #441

Closed
faceyspacey opened this issue Mar 3, 2017 · 7 comments

Comments

@faceyspacey
Copy link

faceyspacey commented Mar 3, 2017

Having consistency with createMemoryHistory is very useful and important. I can only assume there is some challenges to adding entries and index to createBrowserHistory. If not, I'd be more than happy to add it--any direction of where to start and potential pitfalls would be much appreciated.

@faceyspacey
Copy link
Author

faceyspacey commented Mar 3, 2017

I haven't checked the code yet, but it seems the key challenge is when you call pushState when the index is lower than length -1 (i.e. because you've pressed back a few times), in which case you need to set length equal to index + 2 and index to index + 1 while pushing the new entry on to the entries array you're maintaining.

Then it's just listening to onpopstate, and examining the entries array to determine if the user went forward or backward (by comparing newPath === entries[index-1].path || newPath === entries[index+1].path), and then updating the index accordingly.

My hope is there isn't some edge case that breaks the predictability of this, and that's why you didn't do it. Let me know.

@mjackson
Copy link
Member

mjackson commented Mar 3, 2017

This is not possible. The browser maintains its own internal history stack that we have no visibility into.

I'd suggest you take some time and experiment with the browser's history object and try to figure out why the browser itself doesn't expose index or entries. It will help you to understand why we can't do that here.

@mjackson mjackson closed this as completed Mar 3, 2017
@faceyspacey
Copy link
Author

I have. It's completely doable, minus page-refreshes, where we would lose our mirrored history state while the actual browser remembers it. Is that what you are talking about?

@mjackson
Copy link
Member

mjackson commented Mar 4, 2017

Yes, that's exactly what I'm talking about. If you can't persist something like history.entries across a page refresh, it's useless.

@faceyspacey
Copy link
Author

what are your thoughts on performance.navigation and localstorage:

https://developer.mozilla.org/en-US/docs/Web/API/PerformanceNavigation
http://caniuse.com/#feat=nav-timing

if it has enough browser support, which it seems, we can record entries in local storage and know whether to re-use them on next visit if performance.navigation.type > 0

@mjackson
Copy link
Member

mjackson commented Mar 4, 2017

We've tried using sessionStorage in the past to persist data across page refreshes, but ran into lots of different browser quirks that made it unreliable. You can see the hoops we had to jump through in order to use it in the v3 branch.

It could perhaps be supplied as an add-on, but would have all the same caveats as that code.

@faceyspacey
Copy link
Author

Those issues (particularly blocking cookies + private mode in safari) certainly mess things up. And I guess you're right--a use at your own risk addon is the only way to professionally make such functionality available.

Nice work finding those issues by the way.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants