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.

Sign up for GitHub

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

RFC: Track unique history location state #186

Merged
merged 3 commits into from
Jan 20, 2017
Merged

RFC: Track unique history location state #186

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ad4c13
RFC: Track unique history location state
bcardarella Dec 5, 2016
c1fb4cc
Updated from feedback
bcardarella Dec 11, 2016
455ff5f
Change from counter to uuid per Stef's request
bcardarella Jan 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions text/0000-track-unique-history-location-state.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
- Start Date: 2016/12/05
- RFC PR: (leave this empty)
- Ember Issue: (leave this empty)

# Summary

Track unique history location states
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should likely outline the public/intimate API that plugins (or consumers) will use to utilize this feature. This contract/protocol should be clearly defined, as it will be sticking around for some time.


# Motivation

The path alone does not provide enough information. For example, if you
visit page A, scroll down, then click on a link to page B, then click on
a link back to page A. Your actual browser history stack is [A, B, A].
Each of those nodes in the history should have their own unique scroll
position. In order to record this position we need a UUID
for each node in the history.

This API will allow other libraries to reflect upon each location to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seem motivation, not detailed design

determine unique state. For example,
[ember-router-scroll](https://github.com/dollarshaveclub/ember-router-scroll)
is making use of a [modified `Ember.HistoryLocation` object to get this
behavior](https://github.com/dollarshaveclub/ember-router-scroll/blob/master/addon/locations/router-scroll.js).

Tracking unique state is required when setting the scroll position
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seem motivation, not detailed design

properly based upon where you are in the history stack, as described in
[Motivation](#motivation)

# Detailed design

Code: [PR#14011](https://github.com/emberjs/ember.js/pull/14011)

We simply unique identifier (UUID) so we can track uniqueness on two
dimensions. Both `path` and the generated `uuid`. A simple UUID
generator such as
https://gist.github.com/lukemelia/9daf074b1b2dfebc0bd87552d0f6a537
should suffice.

# How We Teach This

We could describe what meta data is generated for each location in the
history stack. For example, it could look like:

```js
// visit page A

[
{ path: '/', uuid: 1 }
]

// visit page B

[
{ path: '/about', uuid: 2 },
{ path: '/', uuid: 1 }
]

// visit page A

[
{ path: '/', uuid: 3 },
{ path: '/about', uuid: 2 },
{ path: '/', uuid: 1 }
]

// click back button

[
{ path: '/about', uuid: 2 },
{ path: '/', uuid: 1 }
]
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is intended for add-on authors or custom location authors. We should likely describe in more detail how to teach them to use the feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm all for that, beyond the the id property being accessible on the history object there isn't anything else being proposed. IMO the easiest example would be to point towards ember-router-scroll's implementation


# Drawbacks

* The property access is writable

# Alternatives

The purpose for this behavior is to enable scroll position libraries.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If all we need is each state to have a unique identifier, could we just use a Map or WeakMap? I believe this may result in no public API change.

const scrollPositions = new Map()

scrollPositions.set(state, [10, 10]);
scrollPositions.set(state2, [100, 100]);

scrollPositions.get(state) // => [10, 10]
scrollPositions.get(state2) // => [100, 100]

If we are worried about a memory leak, a WeakMap could be used instead. Which ember actually has a polyfill that would work for this use case kicking around: https://github.com/emberjs/ember.js/blob/master/features.json#L6

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The state being used by the history location is being set with replaceState https://www.w3.org/TR/2011/WD-html5-20110113/history.html#dom-history-replacestate

I believe it must be a pojo

Copy link
Member

@stefanpenner stefanpenner Dec 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not recommending we change the state object itself, But that we associate data (such as scroll position) with any given state pojo via map or weakmap.

This RFC seems to want to give unique identity to a given state pojo, so that other parts of the system can differentiate between them. As currently described, it suggests branding each pojo with an ID, but why brand at all if we can just use the objects own identity as a way to differentiate. A way to use an objects identity in such a way is a map, where one would use the state pojo as the key in the map. But that may make memory management tricky, hence recommending a weakmap instead.

The only thing I can think of that would prevent this from working, is if the state pojo we push onto the stack, isn't the same pojo (but still contains the same properties/values) we get when we pop off the stack.

Copy link
Member

@stefanpenner stefanpenner Dec 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, i take that back. WeakMap wont work, because state is serialized/deserialized which means when popping it, we get a copy of the original state back. This means, object identity can't be used as key. So branding with something like __id__ or id as you proposed, seems like the correct path.

Sorry for taking you on an adventure, I did not realize that state object was serialized this way. In retrospect it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanpenner does that mean this review is satisfied?

There are two other solutions in the wild. One is in the guides that
suggests resetting the scroll position to `(0, 0)` on each new route
entry. The other is
[ember-memory-scroll](https://github.com/ef4/memory-scroll) which I
believe is better suited for tracking scroll positions for components
rather than the current page.

However, in both cases neither solution provides the experience that
users have come to expect from server-rendered pages. The browser tracks
scroll position and restores it when you revisit the page in the history
stack. The scroll position is unique even if you have multiple instances
of the same page in the stack.

# Unresolved questions

None at this time.