exoscale / coax

Clojure.spec coercion library for clj(s)
Other
118 stars 4 forks source link

Use macrovich to do macrology. #19

Closed jellelicht closed 2 years ago

jellelicht commented 2 years ago

This seems to fix #13 for me

mpenet commented 2 years ago

Hi,

Thanks for looking into this, I have to admit I never spent time to check the cljs side of things in detail.

Would there be a way to achieve the same without using macrovich? I'd like to avoid if possible to add a dependency on it.

mpenet commented 2 years ago

I am not sure, but I think just moving this macro in a separate/dedicated ns (still in cljc) and calling either require or require-macro on it depending on the context from coercer.cljc would fix the issue. Could you confirm this?

mpenet commented 2 years ago

Something like that: https://github.com/exoscale/coax/commit/6b14047d4417504554a3853b21913b63f6654c54

jellelicht commented 2 years ago

It actually does not work properly: Use of undeclared Var exoscale.coax.coercer/java, probably referring to the resolved Exception instead of the nice and tidy :default. I think this happens because the reader conditional in the body of the invalid-on-throw! actually is expanded on the clojure side of things. This is exactly one of the problems that macrovich solves, as per their readme. Observe in exoscale.coax.utils:

(defmacro broken []
  #?(:clj "clojure" :cljs "clojurescript"))

and then in exoscale.coax.coercer:

(defn random-broken []
  (u/broken))

and then from my cljs program/repl:

(random-broken) ; => "clojure"

TLDR: if we want to fix this properly, it should be done in the way that macrovich does it; whether we copy-paste their approach or simply use it as a compile-time dep is of course totally up to you.

mpenet commented 2 years ago

I think we'll just adopt macrovich then. It's a small enough dependency.

mpenet commented 2 years ago

Thanks for the fix, I ll release a new version shortly

mpenet commented 2 years ago

available as 1.0.0-alpha15