Add custom TokenTransformer

andsnpl commented 6 years ago

After encountering #15 I did some debugging and it looks like this occurs because in non-fragment mode, Html5History implements a default token<->url transform that expects token to be derived from the pathname only. When receiving a new token through navigate! that does include query-string, it appends the existing query string, resulting in duplication. See this line in the implementation.

This adds an implementation of the interface defined for overriding that mapping, a public function to instantiate it, and a couple of sentences of documentation.

Tested this in a browser repl on my own machine, but did not write formal tests. I think it needs an integration test running in the browser (or to mock the window object?) Happy to do so, but don't know how that is commonly done in ClojureScript. Examples welcome!

niwinz commented 6 years ago

Nice work, writing integration tests here is complicated, I think that the best (and quick way) is just release a snapshot and let the people try it on their projects.

I'll merge it ASAP. Thanks

andsnpl commented 6 years ago

@niwinz Is that just a matter of changing the version to 1.6.1-SNAPSHOT in project.clj?

andsnpl commented 6 years ago

@niwinz bumping this. Are there any next steps from my end?

niwinz commented 6 years ago

Pushed the snapshot to clojars.

bensu commented 6 years ago

The fix worked for me: I no longer get accumulated query parameters as described in #15

The problem is that in

What I see:

my ClojureScript output directory hast the file bide/impl/TokenTransformer.js in it.
figwheel complains that it can't find

bensu commented 6 years ago

The fix worked for me: I no longer get accumulated query parameters as described in #15

The new problem is that the current snapshot (bide-1.6.1-20180820.103728-1) doesn't compile:

What I see:

my ClojureScript output directory hast the file bide/impl/TokenTransformer.js in it (snake case name)
figwheel complains that it can't find it:

---  Could not Analyze  resources/public/js/web-out/bide/core.cljs  ----

  No such namespace: bide.impl.TokenTransformer, could not locate bide/impl/TokenTransformer.cljs, bide/impl/TokenTransformer.cljc, or JavaScript source providing "bide.impl.TokenTransformer"

----  Analysis Error : Please see resources/public/js/web-out/bide/core.cljs  ----

cljsbuild also can't find it:

Caused by: clojure.lang.ExceptionInfo: No such namespace: bide.impl.TokenTransformer, could not locate bide/impl/TokenTransformer.cljs, bide/impl/TokenTransformer.cljc, or JavaScript source providing "bide.impl.TokenTransformer" in file target/web/bide/core.cljs {:tag :cljs/analysis-error}

I noticed that in the PR the file is spelled in lowercase bide/impl/tokentransformer.js. When I add a copy of that file to my classpath, then figwheel and cljsbuild compile properly, and #15 is properly resolved.
the downloaded jar in ~/.m2 has a lowercase name too. For some reason the build tools are copying it as TokenTransformer.js or not finding it.

It might be a bug of the build tools but I could reproduce the problem and the fix (adding a lowercase copy in my classpath) in both cljsbuild and figwheel. It could also be a bug of the ClojureScript compiler.

bensu commented 6 years ago

Finally, I tried adding bide/impl/TokenTransformer.js to my CircleCI repository and it successfully compiled, while adding bide/impl/tokentransformer.js didn't. I suspect it might be a matter of renaming the file to bide/impl/TokenTransformer.js

niwinz commented 6 years ago

Renamed and pushed new snapshot to clojars.

bensu commented 6 years ago

Thank you! The new snapshot version compiled without problems.

niwinz commented 6 years ago

nice, i will release a new version today probably

funcool / bide

Add custom TokenTransformer #20