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

[Bug]: solid-router-integration: navigation to the same page causes duplication in the router #303

Open
XantreDev opened this issue May 13, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@XantreDev
Copy link

Telegram Application

Telegram Desktop

Describe the Bug

Click to link of the same page causes duplication of history entry (you need to press back button a lot of times without any visual change)

To Reproduce

const Component () => <A href="/self">Link</A>

//routing
<Router>
  <Route component={Component} path="/self" />
  <Route component={Component} path="*" />
</Router>

Expected Behavior

It must stay in the same history item

@XantreDev XantreDev added the bug Something isn't working label May 13, 2024
@XantreDev
Copy link
Author

Here is workaround, I just added equality check to replaceAndMove. @heyqbnk I can provide pr if you want to

diff --git a/dist/index.js b/dist/index.js
index 185b386841dd6e7366403590da75419e7ccaf180..4711ec994aa2252267e3c8f49e9ba69ef29dac97 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -2562,6 +2562,10 @@ class Ye {
     if (!n && this.current === s)
       return;
     const r = this.current;
+    const isLikePrev = (prev, next) => prev.search === next.search && prev.pathname === next.pathname && prev.hash === next.hash
+    if (isLikePrev(s, r)) {
+      return
+    }
     if (this.index !== t) {
       const i = this._index;
       this._index = t, this.attached && i > 0 != t > 0 && this.sync();

@heyqbnk
Copy link
Member

heyqbnk commented May 17, 2024

I don't really remember how it works in browser. Let's imagine, we are currently at the page /page. This page has a link /page (to the same page). What will happen, if user clicks this link? I assume, a new same entry will be added.

We can probably make this behavior optional, providing some option like duplicates: boolean.

@XantreDev
Copy link
Author

XantreDev commented May 17, 2024

We can test it right here
#303 (comment)

It's actually duplicates entry. You're right
But it feels strange for native apps, idk what is better to do with it

@heyqbnk
Copy link
Member

heyqbnk commented May 17, 2024

Seems like one more day in hookah bar should be spent. I will think about a better solution.

Probably, adding duplicates property will not be as useful as adding something like shouldNavigate: (entry: HistoryItem) => boolean. This just gives way more flexibility.

Added this task to the internal dashboard. Will be back with solution soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants
@heyqbnk @XantreDev and others