metosin / malli

High-performance data-driven data specification library for Clojure/Script.
Eclipse Public License 2.0
1.46k stars 208 forks source link

Implement malli.experimental.time schemas for clojurescript using js-joda #853

Closed dvingo closed 1 year ago

dvingo commented 1 year ago

I picked up from the work started by @henryw374 as mentioned in this comment: https://github.com/metosin/malli/issues/49#issuecomment-1384374256 to add time schemas support for clojurescript which match the JVM implementation.

These time namespaces do not affect users who do not require them and thus they also don't need js-joda installed to use malli, unless they're using these schemas of course.

dvingo commented 1 year ago

@Deraen Thank for the review!

There are shadow-cljs builds in the repo but they aren't currently used for testing. I used this namespace during development of malli: https://github.com/dvingo/malli/blob/js-joda/app/malli/instrument_app.cljs with a cljs repl.

So that was how I determined it works with shadow-cljs. I just tried under advanced compilation as well with shadow-cljs and things are working without needing externs.

ikitommi commented 1 year ago

Looks good to me. @Deraen there is one unresolved thread, could you merge after it/this is resolved? Thanks @dvingo!

Deraen commented 1 year ago

The question was answered already.