Router: Active routes push new state to history

[samselikoff]

Just another question :)

I noticed that active routes push new history states to the browser. For example, if you go to https://reach.tech/router, then click Accessibility, then Credits, and then click Credits 2 or 3 more times, then start hitting the back button, the site will stay on Credits. I think each click is pushing new items onto the history stack.

I'm not sure if this behavior is intentional but I found it surprising, as its not how the browser seems to work on server-rendered sites (even if clicking the active link triggers a full-page refresh).

Couldn't find anything in the issues about it so figured I'd kick it off here! Thanks for any info.

[ryanflorence]

Yeah, if you click a normal <a href="something.html"/> without any javascript at all the browser does a replace instead of a push and we should do the same.

The trick is deciding what to do about link clicks with <Link state={...}/> in them. Object identity? Seems annoying to keep that identity around. Shallow equality check? Probably.

[ryanflorence]

@jzabala yeah man!

[ryanflorence]

If you're at the same path, then a push should be a replace, unless there is location state:

<Link to="..." state={...}/>

Options:

1. Always push if there is state.
2. Identity check with ===, replace if same
3. Shallow compare, replace if equal

I think I'd like to go with (2), open to being convinced otherwise.

[ryanflorence]

^ for @jzabala

[jzabala]

Hey thank you very much @ryanflorence. I hope to have this done in no time.
mmm sorry for removing the assignment. It was a mistake 😩. But I am working on this.

[jzabala]

Hey @ryanflorence after reading the code a little bit I think we'll need to go with option (1). The reason is that when we use the navigate function a prop key is being added to the state. So even if the user keep referential identity on her part the state would never be the same.

Any way, this would be the default behavior and if the user wants to change it she just need to send replace along with the state. What do you think?

[jzabala]

If you're at the same path, then a push should be a replace, unless there is location state:

<Link to="..." state={...}/>

Options:

1. Always push if there is state.
2. Identity check with ===, replace if same
3. Shallow compare, replace if equal

I think I'd like to go with (2), open to being convinced otherwise.

And it turns out (3) is the best option here. Option (1) has the problem that it only pushes when the Link has state but, what happens in the case when you go to a page by clicking a Link with state <Link to="..." state={...}/> and later in that page you click a link for the same path without state <Link to="..." /> a replace would be executed replacing the last entry with state. Option (2) as mentioned in my previous comment is not optimal since there would never be referential identity between the states. And that leaves us with option (3).

This shallow compare function from stackoverflow looks good enough:

const shallowCompare = (obj1, obj2) =>
  Object.keys(obj1).length === Object.keys(obj2).length &&
  Object.keys(obj1).every(key => 
    obj2.hasOwnProperty(key) && obj1[key] === obj2[key]
  );

[jzabala]

In the PR https://github.com/reach/router/pull/302 I implemented solution 3. But implementing solution 1 in three steps would be easier:

• If current path and state coming from Link push
• If current path with state (other than key) and no state coming from Link push
• If current path without state (only key) and no state coming from Link replace

Let me know if you prefer that one to change the PR :D