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

Avoid duplicate entries #85

Closed
kenchris opened this issue Mar 23, 2021 · 11 comments
Closed

Avoid duplicate entries #85

kenchris opened this issue Mar 23, 2021 · 11 comments

Comments

@kenchris
Copy link

This might already be covered, but I didn't find it so asking here.

In some SPA experiences you have multiple view that the users often switch between (a bit like browser tabs) - it is kind of time that hitting back brings you to the former view, but it is quite annoying that it cycles through the whole history of clicking, like

Twitter PWA: tweets, dm, tweets, dm, profile, dm, tweets.

I think a nice way to solve this would be to be able to easily avoid having duplicates and thus always remove earlier entries when readding, like the above would become:

profile, dm, tweets

I have wanted to implement such behavior in my own SPAs before

@Yay295
Copy link

Yay295 commented Mar 23, 2021

This sounds like someone is using pushState when they should be using replaceState.

@kenchris
Copy link
Author

That modifies the current history entry, which is not a solution to the above. It is about removing earlier duplicates of a new entry

@tbondwilkinson
Copy link
Contributor

I think a resolution to #9 would solve this use case - namely this would be deleting entries.

As for automatic detection of duplicates and deletion, I would probably say that's too aggressive of a design decision, since there may be some pages where it's reasonable to have two "identical" states in history from the point of view of the URL.

@Yay295
Copy link

Yay295 commented Mar 24, 2021

It is about removing earlier duplicates of a new entry

Using replaceState instead of pushState would mean you wouldn't find yourself in a situation where you have duplicates of previous entries.

@domenic
Copy link
Collaborator

domenic commented Mar 24, 2021

This definitely makes sense, and is not at all related to pushState vs. replaceState.

I tend to agree with @tbondwilkinson that having this as a built-in "mode" is a bit fragile, and it'd be better to build on a primitive like that proposed in #9. Concretely, given something like appHistory.delete(key), you could do something like:

appHistory.addEventListener('currentchange', async () => {
  for (const entry of appHistory.entries) {
    if (isSame(entry, appHistory.current)) {
      await appHistory.delete(entry.key);
    }
  }
});

where isSame could be as simple as comparing the entries' url properties, or more complicated depending on your app's specific logic.

WDYT?

@frehner
Copy link

frehner commented Mar 24, 2021

would it be more consistent to have delete() take in the key for an entry, like navigateTo/goTo does?

@domenic
Copy link
Collaborator

domenic commented Mar 24, 2021

Yep, sorry, that's what I meant to write; I'll edit the code sample.

@kenchris
Copy link
Author

Yes this looks good. Would be great with examples like that in explainer/spec

@domenic
Copy link
Collaborator

domenic commented Mar 24, 2021

Well, we're not yet sure whether mutation of your history list should be supported, so it's currently relegated to an open issue (#9). Maybe I should add a pointer though. And I'll certainly add this as more evidence for #9 being compelling to developers.

@kenchris
Copy link
Author

I definitely think it should be possible to modify it as long as it is from your own origin, so hope that moves forward

@domenic
Copy link
Collaborator

domenic commented Apr 1, 2021

Let me roll this into #9 for now!

@domenic domenic closed this as completed Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants