Telegram-Mini-Apps / telegram-apps

Made from scratch TypeScript packages, examples and documentation you will surely need to start developing on Telegram Mini Apps.
https://docs.telegram-mini-apps.com/
MIT License
657 stars 178 forks source link

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

Open XantreDev opened 5 months ago

XantreDev commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

We can test it right here https://github.com/Telegram-Mini-Apps/tma.js/issues/303#issuecomment-2117437251

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

heyqbnk commented 5 months ago

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