search

domkm / silk

Routing for Clojure & ClojureScript
222 stars 13 forks source link

Can't put RequestMethodPatterns in vars #17

Closed tomconnors closed 8 years ago

tomconnors commented 8 years ago

Hi there, strange problem here. If I eval (domkm.silk.serve/POST) I see the expected {:request-method #object[etc...]} output, but if I then eval (def post-method (domkm.silk.serve/POST)) I get a clojure.lang.Compiler$CompilerException. Same thing happens with the other request methods. Have you seen this behavior? I'm on clojure 1.7.

domkm commented 8 years ago

Why are you evaling it? I don't think you can eval the 1.7 #object notation.

tomconnors commented 8 years ago

Sorry, I only meant that I'm evaluating it at the repl. Not directly calling eval. My routes look something like:

(def routes (silk/routes [[:notify-error ["notify" "error"] {} (domkm.silk.serve/POST)]
                          [:notify-page-view ["notify" "pv"] {} (domkm.silk.serve/POST)]]))

and that won't compile for me. I eventually realized that the part that's not compiling is putting (domkm.silk.serve/POST) in a var. For instance, this compiles just fine:

(silk/routes [[:notify-error ["notify" "error"] {} (domkm.silk.serve/POST)]
             [:notify-page-view ["notify" "pv"] {} (domkm.silk.serve/POST)]])
domkm commented 8 years ago

That is very interesting. defing it causes a CompilerException java.lang.AbstractMethodError. Searching didn't turn up much for me except that this issue can be circumvented by using a local binding.

(let [rts (silk/routes [[:notify-error ["notify" "error"] {} (domkm.silk.serve/POST)]
                        [:notify-page-view ["notify" "pv"] {} (domkm.silk.serve/POST)]])]
  (def routes rts))

It's not ideal but at least it works.

Thanks for bringing this problem to my attention. I will leave this issue open until we have a better solution.

domkm commented 8 years ago

It looks like this is probably because RequestMethodPattern implements clojure.lang.IFn.invoke but not clojure.lang.IFn.applyTo.

See: https://groups.google.com/forum/#!msg/clojure/Fawa2oWWFds/4xUcJ1T8T1AJ

A PR implementing clojure.lang.IFn.applyTo would be very welcome. :)

tomconnors commented 8 years ago

Thanks for the info. You solved my problem. I'll get you that PR soon - looks like all that's needed is:

(applyTo [this args] (clojure.lang.AFn/applyToHelper this args))

though I'll have to see what clojurescript thinks of that. Would you like a test added to the test suite verifying that you can def routes that use :request-method?

domkm commented 8 years ago

Yes, an implementation and test would be great! Thanks.