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

[#203] make sci optional for smaller JS builds #210

Closed
wants to merge 1 commit into from

Conversation

borkdude
Copy link
Contributor

@borkdude borkdude commented Jun 30, 2020

This PR makes sci optional using the same mechanisms that clojure.spec uses to dynamically load test.check. Fixes #203.

  • Maybe split the dynaload code out into a library so it can be used from other projects as well. With copyright notice for code borrowed from spec.
  • Same mechanism can be used to make test.check optional for even smaller builds.
  • The tests do pass but due to randomization in kaocha they sometimes fail. The reason is that sci.core has to be required first. There's probably a kaocha preloads plugin which can do this. But the tests have to be adapted to the optionality of sci anyway.

Note that sci users now have to bring in the sci dependency themselves. If they don't they will get an error message when using malli.core/eval.

$ plk -A:sci
ClojureScript 1.10.597
cljs.user=> (require '[malli.core :as m])
nil
cljs.user=> (require '[sci.core])
nil
cljs.user=> (m/validate int? "1")
false
cljs.user=>
undefined is not an object (evaluating 'malli.core.eval_string.call')
cljs.user=> (m/eval "(+ 1 2 3)")
6

vs.

$ plk
ClojureScript 1.10.597
cljs.user=> (require '[malli.core :as m])
nil
cljs.user=> (m/validate int? "1")
false
cljs.user=> (m/eval "(+ 1 2 3)")
Execution error (Error) at (<cljs repl>:1).
Var sci.core/eval-string does not exist, sci.core never required

@borkdude borkdude marked this pull request as draft June 30, 2020 10:20
@borkdude borkdude closed this Jun 30, 2020
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.

Make SCI dependency optional for smaller JS builds
1 participant