Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

jasonjckn
Copy link
Contributor

@jasonjckn jasonjckn commented Sep 10, 2023

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

@jasonjckn jasonjckn changed the title feat(assert): adds new assert-like functionality to check malli schema DRAFT: feat(assert): adds new assert-like functionality to check malli schema Sep 10, 2023
@jasonjckn jasonjckn changed the title DRAFT: feat(assert): adds new assert-like functionality to check malli schema feat(assert): adds new assert-like functionality to check malli schema Sep 10, 2023
@ikitommi
Copy link
Member

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.

👍

(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: #956

WDYT?

@jasonjckn
Copy link
Contributor Author

jasonjckn commented Sep 25, 2023

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: #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 ikitommi merged commit ab76f78 into metosin:master Oct 31, 2023
9 checks passed
@ikitommi
Copy link
Member

Thank You.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants