search

metosin / malli

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

feat(assert): adds new assert-like functionality to check malli schema #953

Closed jasonjckn closed 8 months ago

jasonjckn commented 10 months ago

First draft of assert functionality,

Couple explanations on approach here (1) the on/off switch for assert checking is modeled off of clojure.core/assert, not clojure.spec.alpha, the latter uses volatile! for on/off switch... the former is more efficient because if assertions are disabled, it won't exist in byte-code. I'm open to changing it, but that's my preference.

(2) I added 'no color' / 'nil color' to virhe, in order to play nicely with emacs which won't render ANSI color codes in stack traces by default.

(3) This is in malli.assert namespace temporarily, just let me know where you want it.

image

ikitommi commented 9 months ago

Hi. Thanks for working on this.

(1) the on/off switch for assert checking is modeled off of clojure.core/assert, not clojure.spec.alpha, the latter uses volatile! for on/off switch... the former is more efficient because if assertions are disabled, it won't exist in byte-code. I'm open to changing it, but that's my preference.

I like this.

(2) I added 'no color' / 'nil color' to virhe, in order to play nicely with emacs which won't render ANSI color codes in stack traces by default.

:+1:

(3) This is in malli.assert namespace temporarily, just let me know where you want it.

I think assert is important and should be in malli.core. Being there, it can't depend on pretty printing, could just use malli.core/coerce under the hood? Just created a separate issue to support pluggable pretty printing in the core: https://github.com/metosin/malli/issues/956

WDYT?

jasonjckn commented 9 months ago

I think assert is important and should be in malli.core. Being there, it can't depend on pretty printing, could just use malli.core/coerce under the hood? Just created a separate issue to support pluggable pretty printing in the core: https://github.com/metosin/malli/issues/956 WDYT?

Agree with everything you wrote,

I ended up calling m/coerce like you suggested, i was trying to think of a way to cache the result of m/coercer in an atom as a performance optimization, but seemed like overkill (?)

i updated the PR, please take a look

ikitommi commented 8 months ago

Thank You.