kibu-australia / pushy

Clojurescript library for quick and easy HTML5 pushState
Eclipse Public License 1.0
223 stars 28 forks source link

Pushy triggering when no URL is specified #10

Closed pupeno closed 9 years ago

pupeno commented 9 years ago

I'm using Pushy with re-frame and I have a link that looks like this:

[:a {:on-click #(re-frame/dispatch [:mark-tool-as-used (:id tool)])} (:name tool)]

It's here: https://github.com/carouselapps/ninjatools/blob/master/src/cljs/ninjatools/views.cljs#L25

I'm expecting that link to just call the ClojureScript method, but it's also triggering Pushy to dispatch. I'm setting it here:

https://github.com/carouselapps/ninjatools/blob/master/src/cljs/ninjatools/routes.cljs#L27

The second dispatch is particularly problematic since the URL being processed is empty.

pupeno commented 9 years ago

I think the problem might be in here:

https://github.com/kibu-australia/pushy/blob/master/src/pushy/core.cljs#L15

My links with no href are not returning null, they are returning "". What do you think about taking "" as no href present? Would that be a breaking change?

pupeno commented 9 years ago

I used this version of processable-url? and that helped with this issue:

(defn processable-url? [uri]
  (and (not (clojure.string/blank? uri))
       (or (not (.hasDomain uri))
           (some? (re-matches (re-pattern (str "^" (.-origin js/location) ".*$"))
                              (str uri))))))

I'd make it the default and I'm happy to submit a patch, but I'm not sure if you have the same opinion.

wavejumper commented 9 years ago

@pupeno looks good. Please submit a pull request!