venantius / accountant

ClojureScript navigation for single-page applications, made simple.
Eclipse Public License 1.0
250 stars 39 forks source link

Problems while using accountant #65

Open pavel-klavik opened 4 years ago

pavel-klavik commented 4 years ago

I am using it together with bidi. In the main function, I am initializing it like this:

(accountant/configure-navigation!
    {:nav-handler       #(rf/dispatch [:app-state/changed-route %])
     :path-exists?      #(contains? #{:app/index :app/help}
                                    (:handler (b/match-route routes/routes %)))
     :reload-same-path? false})

I have several problems with it:

  1. Clicking on back and forward buttons in the browser changes the URL, but does not call :app-state/changed-route event. So it is ignored by the app.

  2. When I remove :reload-same-path?, each click on a link called :app-state/changed-route multiple times. But I want to allow reloading the same path to the user.

  3. Sometimes :app-state/changed-route is called multiple times anyway when I reload the page.

Any idea what I could be doing wrong?

pavel-klavik commented 4 years ago

I did some further digging concerning 2. When I set :reload-same-path? true, then navigating to a different URL dispatches [:app-state/changed-route] twice, while navigating to the same URL dispatches it only once. I believe the reason might be the following part of the code:

(when (not= current-relative-href relative-href) ;; do not add duplicate html5 history state
           (. history (setToken relative-href title)))
         (.preventDefault e)
         (when reload-same-path?
           (events/dispatchEvent history (Event. path true))))))))

When navigating to different URL, both (. history (setToken relative-href title)) and (events/dispatchEvent history (Event. path true)) are called. Is this correct?

pavel-klavik commented 4 years ago

3 is unrelated to accountant.

To solve 1, I have added:

(events/listen "popstate" #(rf/dispatch [:app-state/changed-route
                                         (->> js/document .-location .-href
                                              (re-find #"^.*//[^/]*(/.*)") last)]))

Is this the expected solution. Maybe adding something to the documentation that this has to be done independently?

To solve 2, I did a small hack in which I check in :app-state/changed-route whether routing to the same path did not occur in last 250 ms. But it would be good to avoid this.

KingMob commented 3 years ago

Seconding this. With reload-same-path? set to true, I also get double calls of the nav handler all the time. The issue is the (when reload-same-path? ... doesn't actually check the path. The code should probably be something more like:

(if (and (= current-relative-href relative-href) reload-same-path?)
  (events/dispatchEvent history (Event. path true))
  (. history (setToken relative-href title)))
(.preventDefault e)
iku000888 commented 3 years ago

I don't actively use accountant anymore, but I can merge PRs.