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

Make sci explicitly optional #276

Closed
ikitommi opened this issue Oct 11, 2020 · 1 comment
Closed

Make sci explicitly optional #276

ikitommi opened this issue Oct 11, 2020 · 1 comment
Labels
Clojurists Together Sponsored by Clojurists Together Q3 2020 enhancement New feature or request for discussion Discussion is main focus of PR

Comments

@ikitommi
Copy link
Member

ikitommi commented Oct 11, 2020

Currently, if sci is a transitive dependency or is required by some other component, it will be enabled for Malli too. This is not good. Real solution would be to make installation of sci explicit, e.g. via #235:

1) explicit sci via options

(require '[malli.sci :as ms])
(require '[malli.core :as m])

(def options
  {:evaluator (ms/evaluator)
   :registry (ms/default-registry)})

(def Schema (m/schema [:fn '(fn [x] (string? x))] options))

(m/validate Schema "kikka")
; => true

2) disable via flag

, but, as it would be a breaking change (should wait for 1.0.0?), we could just add a flag wether to use sci or not.

(m/schema [:fn '(fn [x] (string? x))] {::m/enable-sci false})
; Exectution error: :malli.core/sci-not-available {:code (fn [x] (string? x))}
@ikitommi ikitommi added Clojurists Together Sponsored by Clojurists Together Q3 2020 enhancement New feature or request for discussion Discussion is main focus of PR labels Oct 11, 2020
ikitommi added a commit that referenced this issue Oct 11, 2020
ikitommi added a commit that referenced this issue Oct 11, 2020
@ikitommi
Copy link
Member Author

... and why is it not good? currently, you can't force disable sci - if you are reading schemas from 3rd party, they can attach any sci-script it, see also: babashka/sci#348

ikitommi added a commit that referenced this issue Oct 12, 2020
explicitly disabling sci & sci options, fixes #276
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clojurists Together Sponsored by Clojurists Together Q3 2020 enhancement New feature or request for discussion Discussion is main focus of PR
Projects
None yet
Development

No branches or pull requests

1 participant