From c1dcd7a76404bed90a5c9b6913a5ef8016fc1382 Mon Sep 17 00:00:00 2001 From: Icaro Motta Date: Sat, 18 Nov 2023 11:04:48 -0300 Subject: [PATCH] Introduce malli library (#17867) This commit is the foundational step to start using malli (https://github.com/metosin/malli) in this project. Take in consideration we will only be able to realize malli's full power in future iterations. For those without context: the mobile team watched a presentation about malli and went through a light RFC to put everyone on the same page, among other discussions here and there in PRs. To keep things relatively short: 1. Unit, integration and component tests will short-circuit (fail) when inputs/outputs don't conform to their respective function schemas (CI should fail too). 2. Failed schema checks will not block the app from initializing, nor throw an exception that would trigger the LogBox. Exceptions are only thrown in the scope of automated tests. 3. There's zero performance impact in production code because we only instrument. Instrumentation is removed from the compiled code due to the usage of "^boolean js.goog/DEBUG". 4. We shouldn't expect any meaningful slowdown during development. **What are we instrumenting in this PR?** Per our team's agreement, we're only instrumenting the bare minimum to showcase 2 examples. - Instrument a utility function utils.money/format-amount using the macro approach. - Instrument a quo component quo.components.counter.step.view/view using the functional approach. Both approaches are useful, the functional approach is powerful and allow us to instrument anonymous functions, like the ones we pass to subscriptions or event handlers, or the higher-order function quo.theme/with-theme. The macro approach is perfect for functions already defined with defn. **I evaluated the schema or function in the REPL but nothing changes** - If you evaluate the source function, you need to evaluate schema/=> or schema/instrument as well. - Remember to *var quote* when using schema/instrument. - You must call "(status-im2.setup.schema/setup!)" after any var is re-instrumented. It's advisable to add a keybinding in your editor to send this expression automatically to the CLJS REPL, or add the call at the end of the namespace you are working on (similar to how some devs add "(run-tests)" at the end of test namespaces). **Where should schemas be defined?** For the moment, we should focus on instrumenting quo components, so define each function schema in the same namespace as the component's public "view" var. To be specific: - A schema used only to instrument a single function and not used elsewhere, like a quo component schema, wouldn't benefit from being defined in a separate namespace because that would force the developer to constantly open two files instead of one to check function signatures. - A common schema reused across the repo, like ":schema.common/theme" should be registered in the global registry "schema.registry" so that consumers can just refer to it by keyword, as if it was a built-in malli schema. - A common schema describing status-go entities like message, notification, community, etc can be stored either in the respective "src/status_im2/contexts/*" or registered globally, or even somewhere else. This is yet to be defined, but since I chose not to include schemas for them, we can postpone this guideline. --- .clj-kondo/config.edn | 16 ++- .clj-kondo/metosin/malli/config.edn | 2 + .zprintrc | 1 + nix/deps/clojure/deps.json | 60 ++++++++-- nix/deps/clojure/deps.list | 12 +- shadow-cljs.edn | 20 +++- .../counter/step/component_spec.cljs | 10 +- src/quo/components/counter/step/view.cljs | 23 +++- .../markdown/list/component_spec.cljs | 7 +- src/quo/core.cljs | 2 +- src/schema/README.md | 68 ++++++++++++ src/schema/common.cljs | 14 +++ src/schema/core.clj | 17 +++ src/schema/core.cljs | 95 ++++++++++++++++ src/schema/registry.cljs | 26 +++++ src/schema/state.cljs | 11 ++ src/schema/style.cljs | 28 +++++ src/schema/view.cljs | 18 +++ src/status_im/test_runner.cljs | 2 + .../contexts/quo_preview/counter/step.cljs | 2 +- src/status_im2/navigation/view.cljs | 5 +- src/status_im2/setup/dev.cljs | 16 +-- src/status_im2/setup/hot_reload.cljs | 6 +- src/status_im2/setup/schema.cljs | 104 ++++++++++++++++++ src/status_im2/setup/schema_preload.cljs | 6 + src/utils/money.cljs | 5 + test/jest/jest.config.js | 6 +- 27 files changed, 539 insertions(+), 43 deletions(-) create mode 100644 .clj-kondo/metosin/malli/config.edn create mode 100644 src/schema/README.md create mode 100644 src/schema/common.cljs create mode 100644 src/schema/core.clj create mode 100644 src/schema/core.cljs create mode 100644 src/schema/registry.cljs create mode 100644 src/schema/state.cljs create mode 100644 src/schema/style.cljs create mode 100644 src/schema/view.cljs create mode 100644 src/status_im2/setup/schema.cljs create mode 100644 src/status_im2/setup/schema_preload.cljs diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index 285be701a8..498e119204 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -16,10 +16,18 @@ :clj-kondo-config {:level :error} :cond-else {:level :error} :consistent-alias {:level :error - :aliases {clojure.string string - clojure.set set - clojure.walk walk - taoensso.timbre log}} + :aliases {clojure.set set + clojure.string string + clojure.walk walk + malli.core malli + malli.dev.pretty malli.pretty + malli.dev.virhe malli.virhe + malli.error malli.error + malli.generator malli.generator + malli.transform malli.transform + malli.util malli.util + schema.core schema + taoensso.timbre log}} :deprecated-namespace {:level :warning} :docstring-blank {:level :error} :equals-true {:level :error} diff --git a/.clj-kondo/metosin/malli/config.edn b/.clj-kondo/metosin/malli/config.edn new file mode 100644 index 0000000000..0f8b25ccfd --- /dev/null +++ b/.clj-kondo/metosin/malli/config.edn @@ -0,0 +1,2 @@ +{:lint-as {malli.experimental/defn schema.core/defn} + :linters {:unresolved-symbol {:exclude [(malli.core/=>)]}}} diff --git a/.zprintrc b/.zprintrc index 5be832ad4b..9f4c2bd0c5 100644 --- a/.zprintrc +++ b/.zprintrc @@ -36,6 +36,7 @@ "deftest-sub" :arg1-body "wait-for" :arg1-body "with-deps-check" :arg1-body + "schema/=>" :arg1-body "->" [:noarg1-body {:list {:constant-pair? false :force-nl? false} :next-inner-restore [[:list :constant-pair?]]}] diff --git a/nix/deps/clojure/deps.json b/nix/deps/clojure/deps.json index 9fdc0e508d..628cd2bd2c 100644 --- a/nix/deps/clojure/deps.json +++ b/nix/deps/clojure/deps.json @@ -45,11 +45,20 @@ }, { - "path": "borkdude/edamame/1.1.17/edamame-1.1.17", + "path": "borkdude/dynaload/0.3.5/dynaload-0.3.5", "host": "https://repo.clojars.org", "jar": { - "sha1": "9087f7abf0104e0354d7db7fc4576608eac558f4", - "sha256": "1n1872i240lakn4pzsag4grf7bv7lcsipmqllxd9m4k1zp3dgla1" + "sha1": "accd696ba364b850b4d92e38f5a34d0e828a0ad1", + "sha256": "0k62m1f29xfh3cp67w7kcvkp5aj35simi8kf95ycvkmgp76w11q8" + } + }, + + { + "path": "borkdude/edamame/1.3.23/edamame-1.3.23", + "host": "https://repo.clojars.org", + "jar": { + "sha1": "254d023e97ed438f0f44532b5a06d928d031ede4", + "sha256": "0l7mxza2nimslhg0qh6jp7gmb8sd6l89fk5d1zvzq2xskscq2vly" } }, @@ -81,20 +90,20 @@ }, { - "path": "cider/cider-nrepl/0.29.0/cider-nrepl-0.29.0", + "path": "cider/cider-nrepl/0.25.3/cider-nrepl-0.25.3", "host": "https://repo.clojars.org", "jar": { - "sha1": "45f6034b26a14138e74145b7a4059628c0fedcd1", - "sha256": "1dy1l6y8cb8xiqq97a4lf8giyiicq4wfl4s2lxn5fb6614cjxqx2" + "sha1": "5ae0efd9377a5e60c084bdaf4a2ce094f759ce23", + "sha256": "0drxf9nm23i1pcgrkwbcr09msq37csilzww38709add0hz8spjhq" } }, { - "path": "cider/piggieback/0.5.2/piggieback-0.5.2", + "path": "cider/piggieback/0.4.1/piggieback-0.4.1", "host": "https://repo.clojars.org", "jar": { - "sha1": "ecfd5c286a85db3f059e75c37fca5722d9e26f79", - "sha256": "1ps9yf3cxmlm447hqkidjb5xry90n0wl3jk0jn28fagq31lzylkl" + "sha1": "0a02a3e2ecd7a126ab60d8a44793342f20ced79b", + "sha256": "142vl5np33akcrnn6pksi0rjfsmmi528villxsj6cwcndvybiw4m" } }, @@ -539,6 +548,24 @@ } }, + { + "path": "metosin/malli/0.13.0/malli-0.13.0", + "host": "https://repo.clojars.org", + "jar": { + "sha1": "286c52d26a3a9d613b5f692971bd19c581cdc4b0", + "sha256": "0vaq9d6cln5j4ww302fwlgg7m647cf2j16gs2i1p1d4klbn4jddz" + } + }, + + { + "path": "mvxcvi/arrangement/2.1.0/arrangement-2.1.0", + "host": "https://repo.clojars.org", + "jar": { + "sha1": "1bc8f3bba7a28de85f319b7d464aa8d955f44918", + "sha256": "1j1rkwrs4wm8zk9v7ilgpnyav5sipdd2bmb83sazh392blw52wpk" + } + }, + { "path": "net/cgrand/macrovich/0.2.1/macrovich-0.2.1", "host": "https://repo.clojars.org", @@ -728,6 +755,15 @@ } }, + { + "path": "org/clojure/test.check/1.1.1/test.check-1.1.1", + "host": "https://repo1.maven.org/maven2", + "jar": { + "sha1": "f33d988fd57bc9c11af1952db81c10f319c91416", + "sha256": "0y2hpkj7zl4yrpsl35ifpdaja5c72b8fpjcnmdgmld9c7cb1hlcl" + } + }, + { "path": "org/clojure/tools.analyzer/1.1.0/tools.analyzer-1.1.0", "host": "https://repo1.maven.org/maven2", @@ -909,11 +945,11 @@ }, { - "path": "refactor-nrepl/refactor-nrepl/3.6.0/refactor-nrepl-3.6.0", + "path": "refactor-nrepl/refactor-nrepl/2.5.0/refactor-nrepl-2.5.0", "host": "https://repo.clojars.org", "jar": { - "sha1": "2b3bb82da53b5db9c2b2aa298417816b81d0ed97", - "sha256": "1ysqabmlnghki6x0636zngxza2d83c85276wp9ma9wk183mkv52a" + "sha1": "6bc3441afc94f7ca024e41a864ca75e05df7e207", + "sha256": "0w8hax99y98l53mixxzx2ja0vcnhjv8dnsaz1zj3vqk775ns5w6i" } }, diff --git a/nix/deps/clojure/deps.list b/nix/deps/clojure/deps.list index eb31303723..3e0fb09d30 100644 --- a/nix/deps/clojure/deps.list +++ b/nix/deps/clojure/deps.list @@ -3,12 +3,13 @@ babashka/fs/0.2.16/fs-0.2.16.jar bidi/bidi/2.1.6/bidi-2.1.6.jar binaryage/env-config/0.2.2/env-config-0.2.2.jar binaryage/oops/0.7.2/oops-0.7.2.jar -borkdude/edamame/1.1.17/edamame-1.1.17.jar +borkdude/dynaload/0.3.5/dynaload-0.3.5.jar +borkdude/edamame/1.3.23/edamame-1.3.23.jar borkdude/sci.impl.reflector/0.0.1/sci.impl.reflector-0.0.1.jar camel-snake-kebab/camel-snake-kebab/0.4.3/camel-snake-kebab-0.4.3.jar cheshire/cheshire/5.11.0/cheshire-5.11.0.jar -cider/cider-nrepl/0.29.0/cider-nrepl-0.29.0.jar -cider/piggieback/0.5.2/piggieback-0.5.2.jar +cider/cider-nrepl/0.25.3/cider-nrepl-0.25.3.jar +cider/piggieback/0.4.1/piggieback-0.4.1.jar clj-kondo/clj-kondo/2023.09.07/clj-kondo-2023.09.07.jar cljs-bean/cljs-bean/1.9.0/cljs-bean-1.9.0.jar clout/clout/2.1.2/clout-2.1.2.jar @@ -58,6 +59,8 @@ javax/annotation/jsr250-api/1.0/jsr250-api-1.0.jar javax/servlet/servlet-api/2.5/servlet-api-2.5.jar javax/xml/bind/jaxb-api/2.3.0/jaxb-api-2.3.0.jar medley/medley/0.8.2/medley-0.8.2.jar +metosin/malli/0.13.0/malli-0.13.0.jar +mvxcvi/arrangement/2.1.0/arrangement-2.1.0.jar net/cgrand/macrovich/0.2.1/macrovich-0.2.1.jar net/java/dev/jna/jna/5.12.1/jna-5.12.1.jar nrepl/bencode/1.1.0/bencode-1.1.0.jar @@ -79,6 +82,7 @@ org/clojure/data.priority-map/1.1.0/data.priority-map-1.1.0.jar org/clojure/google-closure-library/0.0-20230227-c7c0a541/google-closure-library-0.0-20230227-c7c0a541.jar org/clojure/google-closure-library-third-party/0.0-20230227-c7c0a541/google-closure-library-third-party-0.0-20230227-c7c0a541.jar org/clojure/spec.alpha/0.3.218/spec.alpha-0.3.218.jar +org/clojure/test.check/1.1.1/test.check-1.1.1.jar org/clojure/tools.analyzer/1.1.0/tools.analyzer-1.1.0.jar org/clojure/tools.analyzer.jvm/1.2.2/tools.analyzer.jvm-1.2.2.jar org/clojure/tools.cli/1.0.206/tools.cli-1.0.206.jar @@ -99,7 +103,7 @@ org/wildfly/common/wildfly-common/1.5.2.Final/wildfly-common-1.5.2.Final.jar prismatic/schema/1.1.7/schema-1.1.7.jar reagent/reagent/1.2.0/reagent-1.2.0.jar re-com/re-com/2.8.0/re-com-2.8.0.jar -refactor-nrepl/refactor-nrepl/3.6.0/refactor-nrepl-3.6.0.jar +refactor-nrepl/refactor-nrepl/2.5.0/refactor-nrepl-2.5.0.jar re-frame/re-frame/1.3.0/re-frame-1.3.0.jar re-frisk-remote/re-frisk-remote/1.6.0/re-frisk-remote-1.6.0.jar re-frisk/sente/1.15.0/sente-1.15.0.jar diff --git a/shadow-cljs.edn b/shadow-cljs.edn index 4896af3437..47e927e096 100644 --- a/shadow-cljs.edn +++ b/shadow-cljs.edn @@ -9,6 +9,7 @@ [cljs-bean "1.9.0"] [com.cognitect/transit-cljs "0.8.280"] [camel-snake-kebab "0.4.3"] + [metosin/malli "0.13.0"] ;; Dev dependencies [refactor-nrepl "2.5.0"] @@ -50,6 +51,7 @@ :after-load-async status-im2.setup.hot-reload/reload :build-notify status-im2.setup.hot-reload/build-notify :preloads [re-frisk-remote.preload + status-im2.setup.schema-preload ;; In order to use component test helpers in the REPL we ;; need to preload namespaces that are not normally required ;; by production code, such as @@ -71,6 +73,10 @@ :warnings {:fn-deprecated false} :closure-defines {re-frame.trace/trace-enabled? true} :source-map false + ;; This seems to be necessary while using the REPL, + ;; otherwise sometimes you'll get weird errors when + ;; instrumenting functions. + :static-fns false :infer-externs true} ;; if you want to use a real device, set your local ip ;; in the SHADOW_HOST env variable to make sure that @@ -104,13 +110,15 @@ :output-dir "target/test" :optimizations :simple :target :node-test + :dev {:devtools {:preloads [status-im2.setup.schema-preload]}} ;; Uncomment line below to `make test-watch` a specific file ;; :ns-regexp "status-im2.subs.messages-test$" :main status-im.test-runner/main ;; set :ui-driven to true to let shadow-cljs inject node-repl :ui-driven true :closure-defines - {status-im2.config/POKT_TOKEN #shadow/env "POKT_TOKEN" + {schema.core/throw-on-error? true + status-im2.config/POKT_TOKEN #shadow/env "POKT_TOKEN" status-im2.config/INFURA_TOKEN #shadow/env "INFURA_TOKEN" status-im2.config/OPENSEA_API_KEY #shadow/env "OPENSEA_API_KEY" status-im2.config/ALCHEMY_ARBITRUM_GOERLI_TOKEN #shadow/env "ALCHEMY_ARBITRUM_GOERLI_TOKEN" @@ -139,9 +147,17 @@ :compiler-options {:optimizations :simple :source-map false}} :component-test {:target :npm-module - :entries [quo.core-spec status-im2.core-spec] + :entries [;; We need to tell shadow-cljs to compile + ;; the preloads namespace because it will + ;; be used directly by Jest in the option + ;; setupFilesAfterEnv. + status-im2.setup.schema-preload + + quo.core-spec + status-im2.core-spec] :ns-regexp "component-spec$" :output-dir "component-spec" + :closure-defines {schema.core/throw-on-error? true} :compiler-options {:warnings-as-errors false :static-fns false :infer-externs true}}}} diff --git a/src/quo/components/counter/step/component_spec.cljs b/src/quo/components/counter/step/component_spec.cljs index 5a7f11a857..32be19bd84 100644 --- a/src/quo/components/counter/step/component_spec.cljs +++ b/src/quo/components/counter/step/component_spec.cljs @@ -3,18 +3,22 @@ [quo.components.counter.step.view :as step] [test-helpers.component :as h])) +(defn render + [component] + (h/render-with-theme-provider component :dark)) + (h/describe "step component" (h/test "default render of step component" - (h/render [step/view {} nil]) + (render [step/view {} nil]) (-> (h/expect (h/query-by-label-text :step-counter)) (h/is-truthy))) (h/test "renders step with a string value" - (h/render [step/view {} "1"]) + (render [step/view {} "1"]) (-> (h/expect (h/get-by-text "1")) (h/is-truthy))) (h/test "renders step with an integer value" - (h/render [step/view {} 1]) + (render [step/view {} 1]) (-> (h/expect (h/get-by-text "1")) (h/is-truthy)))) diff --git a/src/quo/components/counter/step/view.cljs b/src/quo/components/counter/step/view.cljs index a6423257b0..2a5797865e 100644 --- a/src/quo/components/counter/step/view.cljs +++ b/src/quo/components/counter/step/view.cljs @@ -2,10 +2,24 @@ (:require [quo.components.counter.step.style :as style] [quo.components.markdown.text :as text] - [quo.theme :as theme] + quo.theme [react-native.core :as rn] + [schema.core :as schema] [utils.number])) +(def ?schema + [:=> + [:catn + [:props + [:map {:closed true} + [:accessibility-label {:optional true} [:maybe :keyword]] + [:customization-color {:optional true} [:maybe :schema.common/customization-color]] + [:in-blur-view? {:optional true} [:maybe :boolean]] + [:theme :schema.common/theme] + [:type {:optional true} [:enum :active :complete :neutral]]]] + [:value [:maybe [:or :string :int]]]] + :any]) + (defn- view-internal [{:keys [type accessibility-label theme in-blur-view? customization-color]} value] (let [type (or type :neutral) @@ -23,6 +37,9 @@ [text/text {:weight :medium :size :label - :style {:color (style/text-color type theme)}} label]])) + :style {:color (style/text-color type theme)}} + label]])) -(def view (theme/with-theme view-internal)) +(def view + (quo.theme/with-theme + (schema/instrument #'view-internal ?schema))) diff --git a/src/quo/components/markdown/list/component_spec.cljs b/src/quo/components/markdown/list/component_spec.cljs index 33e2d45943..10838b3158 100644 --- a/src/quo/components/markdown/list/component_spec.cljs +++ b/src/quo/components/markdown/list/component_spec.cljs @@ -22,9 +22,10 @@ (h/is-truthy (h/get-by-text "test description"))) (h/test "renders step component when step-number is valid and type is step" - (h/render [list/view - {:type :step - :step-number 1}]) + (h/render-with-theme-provider [list/view + {:type :step + :step-number 1}] + :dark) (h/is-truthy (h/get-by-label-text :step-counter))) (h/test "renders decription with a context tag component and description after the tag" diff --git a/src/quo/core.cljs b/src/quo/core.cljs index 4ca0c95053..55579fc87a 100644 --- a/src/quo/core.cljs +++ b/src/quo/core.cljs @@ -212,7 +212,7 @@ ;;;; Counter (def counter quo.components.counter.counter.view/view) -(def step quo.components.counter.step.view/view) +(def step #'quo.components.counter.step.view/view) ;;;; Dividers (def divider-label quo.components.dividers.divider-label.view/view) diff --git a/src/schema/README.md b/src/schema/README.md new file mode 100644 index 0000000000..7cc3e698b5 --- /dev/null +++ b/src/schema/README.md @@ -0,0 +1,68 @@ +# Schemas + +This document will grow to describe how we use Malli in the project and our +conventions. It's still early days 🐪 + +## Guidelines +### Use var quote `#'` when aliasing instrumented vars + +It is common in this repository to have aliases to vars. For example, `view` +referring to `var-internal`, or `quo.core/button` referring to +`quo.components.buttons.button.view`. + +If the original var being aliased is instrumented, the alias var MUST [var +quote](https://clojure.org/guides/weird_characters#_var_quote) the original var. +If you don't do this, the aliased var will not be instrumented. + +```clojure +;; bad, view-internal is instrumented, but both aliases don't use a var quote. +(schema.core/=> view-internal ?schema) +(def view (quo.theme/with-theme view-internal)) +(def button quo.components.buttons.button.view/button) + +;; good +(schema.core/=> view-internal ?schema) +(def view (quo.theme/with-theme #'view-internal)) +(def button #'quo.components.buttons.button.view/button) +``` + +### Prefix schema references with `?` + +Prefix schema bindings and vars with a question mark `?`. This is the naming +convention used by malli itself when functions receive instances of schemas and +it's an unambiguous way to avoid naming clashes. + +```clojure +;; bad +(def message-type [:enum ...]) + +(defn view + [message-type] ; Shadows `message-type` schema + (do-something message-type)) + +;; good +(def ?message-type [:enum ...]) + +(defn view + [message-type] ; Unambiguous naming strategy + (do-something message-type)) +``` + +### Define schemas as functions when needed + +Malli has many utility functions to manipulate schemas as data, and they will +automatically check if the schemas were already defined in the registry. + +For schemas we want to conveniently access from the global registry, like +`:schema.common/theme`, they must be registered before Malli tries to use them. + +```clojure +;; bad, will fail if :schema.common/bar is not registered. +(def ^:private ?foo + (malli.util/select-keys :schema.common/bar [:id :name])) + +;; good, execution will be delayed until the schema ?foo is correctly registered. +(defn- ?foo + [] + (malli.util/select-keys :schema.common/bar [:id :name])) +``` diff --git a/src/schema/common.cljs b/src/schema/common.cljs new file mode 100644 index 0000000000..ef6ae00fcb --- /dev/null +++ b/src/schema/common.cljs @@ -0,0 +1,14 @@ +(ns schema.common + (:require + [schema.registry :as registry])) + +(def ^:private ?theme + [:enum :light :dark]) + +(def ^:private ?customization-color + [:or :string :keyword]) + +(defn register-schemas + [] + (registry/register ::theme ?theme) + (registry/register ::customization-color ?customization-color)) diff --git a/src/schema/core.clj b/src/schema/core.clj new file mode 100644 index 0000000000..1a4762cee3 --- /dev/null +++ b/src/schema/core.clj @@ -0,0 +1,17 @@ +(ns schema.core) + +(defmacro => + "Similar to `malli.core/=>`, but instrumentation code will be completely removed + in non-debug environments. + + `value` is first transformed via `malli.core/schema` to make sure we fail fast + and only register valid schemas." + [sym value] + `(if ^boolean js/goog.DEBUG + (try + (malli.core/=> ~sym (malli.core/schema ~value)) + (catch js/Error e# + (taoensso.timbre/error "Failed to instrument function" + {:symbol ~sym :error e#}) + ~sym)) + ~sym)) diff --git a/src/schema/core.cljs b/src/schema/core.cljs new file mode 100644 index 0000000000..69e31ab7e6 --- /dev/null +++ b/src/schema/core.cljs @@ -0,0 +1,95 @@ +(ns schema.core + (:require-macros schema.core) + (:require + [malli.core :as malli] + [malli.dev.pretty :as malli.pretty] + schema.state + [taoensso.timbre :as log])) + +(goog-define throw-on-error? false) + +(defn- ui-reporter + "Prints to STDOUT and signals a schema error should be displayed on screen." + [schema-id printer] + (let [report (malli.pretty/reporter printer)] + (fn [type data] + (report type data) + (swap! schema.state/errors conj schema-id)))) + +(defn- thrower + "Similar to `malli.dev.pretty/thrower`, but this reporter uses js/Error instead + of ex-info, otherwise invalid schema errors will be printed in one long and + incomprehensible line in unit test failures." + ([] (thrower (malli.pretty/-printer))) + ([printer] + (let [report (malli.pretty/reporter printer)] + (fn [type data] + (let [message (with-out-str (report type data))] + (throw (js/Error. (str "\n" message)))))))) + +(defn reporter + ([] + (reporter nil nil)) + ([schema-id] + (reporter schema-id nil)) + ([schema-id opts] + (let [printer (malli.pretty/-printer + (merge {:width 60 + :print-length 6 + :print-level 3 + :print-meta false} + opts))] + (if throw-on-error? + ;; The thrower reporter should be used to short-circuit tests on schema errors. + (thrower printer) + (ui-reporter schema-id printer))))) + +(defn- with-clear-schema-error + "Clears current schema error on app if args passed to `f` are valid." + [schema-id validate-input f] + (fn [& args] + (when (validate-input args) + (swap! schema.state/errors disj schema-id)) + (apply f args))) + +(defn instrument + "Similar to `malli/-instrument`, but will automatically clear up visible schema + errors if `f` is called with valid arguments. + + We use a validator cached by `malli.core/validator`, so that validation is + performed once. + + If `?schema` is invalid, then behave like a nop, log the error and return `f`. + + `schema-id` is optional, but should be passed when instrumenting anonymous + functions. Consider using a namespaced keyword. For example: + + (schema/instrument ::foo + (fn [x] (inc x)) + [:=> [:cat :int] :int]) + " + ([f ?schema] + (if ^boolean js/goog.DEBUG + (let [schema-id (when (var? f) (symbol f))] + (when-not schema-id + (log/warn "Anonymous function instrumented without an explicit identifier." + {:schema ?schema})) + (instrument schema-id f ?schema)) + f)) + ([schema-id f ?schema] + (if ^boolean js/goog.DEBUG + (try + (let [?schema (malli/schema ?schema) + {schema-input :input} (malli/-function-info ?schema) + [validate-input _] (malli/-vmap malli/validator [schema-input])] + (malli/-instrument {:schema ?schema + :report (reporter schema-id + {:title (str "Schema error at " schema-id)})} + (with-clear-schema-error schema-id validate-input f))) + (catch js/Error e + (log/error "Failed to instrument function" + {:schema-id schema-id + :error e + :function f}) + f)) + f))) diff --git a/src/schema/registry.cljs b/src/schema/registry.cljs new file mode 100644 index 0000000000..6af72064f4 --- /dev/null +++ b/src/schema/registry.cljs @@ -0,0 +1,26 @@ +(ns schema.registry + (:refer-clojure :exclude [merge]) + (:require + [malli.core :as malli] + malli.registry)) + +(defonce ^:private registry + (atom (malli/default-schemas))) + +(defn init-global-registry + [] + (malli.registry/set-default-registry! (malli.registry/mutable-registry registry))) + +(defn register + "Defines a new schema in mutable `registry`. + + We normalize `?schema` by always registering it as a proper instance of + `malli.core/Schema` to avoid inconsistencies down the road." + [type ?schema] + (swap! registry assoc type (malli/schema ?schema)) + ?schema) + +(defn merge + [& schemas] + (apply swap! registry cljs.core/merge schemas) + schemas) diff --git a/src/schema/state.cljs b/src/schema/state.cljs new file mode 100644 index 0000000000..cd46cc64a4 --- /dev/null +++ b/src/schema/state.cljs @@ -0,0 +1,11 @@ +(ns schema.state + (:require [reagent.core :as reagent])) + +(def errors + "Set of schema identifiers, usually namespaced keywords. When the set is empty, + no schema errors will be displayed on the app. See `schema.view/view`." + (reagent/atom #{})) + +(defn clear-errors + [] + (reset! errors #{})) diff --git a/src/schema/style.cljs b/src/schema/style.cljs new file mode 100644 index 0000000000..f782de037f --- /dev/null +++ b/src/schema/style.cljs @@ -0,0 +1,28 @@ +(ns schema.style) + +(defn container + [{:keys [bottom-inset]}] + {:align-items :center + :flex-direction :row + :background-color "#cc0000" + :border-bottom-left-radius 8 + :border-top-left-radius 8 + :justify-content :center + :padding-vertical 6 + :padding-right 16 + :position :absolute + :bottom (+ 12 bottom-inset) + :right 0 + :z-index 10000000}) + +(def icon + {:margin-horizontal 8}) + +(def text + {:font-family "Inter-SemiBold" + :font-size 13 + :color "#ddd"}) + +(def text-suffix + {:font-style :italic + :font-size 9}) diff --git a/src/schema/view.cljs b/src/schema/view.cljs new file mode 100644 index 0000000000..6ff2c1074c --- /dev/null +++ b/src/schema/view.cljs @@ -0,0 +1,18 @@ +(ns schema.view + (:require + [quo.core :as quo] + [react-native.core :as rn] + [react-native.safe-area :as safe-area] + schema.state + [schema.style :as style])) + +(defn view + [] + (when (seq @schema.state/errors) + [rn/pressable + {:on-press schema.state/clear-errors + :style (style/container {:bottom-inset (safe-area/get-bottom)})} + [quo/icon :i/close {:size 12 :color "#ddd" :container-style style/icon}] + [rn/text {:style style/text} + "Schema error(s)" + [rn/text {:style (merge style/text style/text-suffix)} " check logs"]]])) diff --git a/src/status_im/test_runner.cljs b/src/status_im/test_runner.cljs index ee6f89c0be..616c9ccbe9 100644 --- a/src/status_im/test_runner.cljs +++ b/src/status_im/test_runner.cljs @@ -8,6 +8,7 @@ status-im.subs.root status-im2.setup.i18n-resources [status-im2.setup.interceptors :as interceptors] + [status-im2.setup.schema :as schema] status-im2.subs.root [utils.re-frame :as rf])) @@ -118,6 +119,7 @@ [& args] (reset-test-data!) (interceptors/register-global-interceptors) + (schema/setup!) (rf/set-mergeable-keys #{:filters/load-filters :pairing/set-installation-metadata :dispatch-n diff --git a/src/status_im2/contexts/quo_preview/counter/step.cljs b/src/status_im2/contexts/quo_preview/counter/step.cljs index d4a89920bc..e22635eb5b 100644 --- a/src/status_im2/contexts/quo_preview/counter/step.cljs +++ b/src/status_im2/contexts/quo_preview/counter/step.cljs @@ -26,4 +26,4 @@ :descriptor descriptor :blur? (:in-blur-view? @state) :show-blur-background? (:in-blur-view? @state)} - [quo/step @state (:value @state)]]))) + [quo/step (dissoc @state :value) (:value @state)]]))) diff --git a/src/status_im2/navigation/view.cljs b/src/status_im2/navigation/view.cljs index 515f93c669..d008cd4685 100644 --- a/src/status_im2/navigation/view.cljs +++ b/src/status_im2/navigation/view.cljs @@ -5,6 +5,7 @@ [react-native.core :as rn] [react-native.safe-area :as safe-area] [reagent.core :as reagent] + schema.view [status-im.bottom-sheet.sheets :as bottom-sheets-old] [status-im.ui.screens.popover.views :as popover] [status-im.ui.screens.profile.visibility-status.views :as visibility-status-views] @@ -71,7 +72,9 @@ [bottom-sheet-screen/view {:content component}] [component])] (when js/goog.DEBUG - [reloader/reload-view])])))) + [:<> + [reloader/reload-view] + [schema.view/view]])])))) (def bottom-sheet (reagent/reactify-component diff --git a/src/status_im2/setup/dev.cljs b/src/status_im2/setup/dev.cljs index 5b34e546bf..dae41e8a16 100644 --- a/src/status_im2/setup/dev.cljs +++ b/src/status_im2/setup/dev.cljs @@ -2,6 +2,7 @@ (:require ["react-native" :refer (DevSettings LogBox)] [react-native.platform :as platform] + [status-im2.setup.schema :as schema] [utils.re-frame :as rf])) ;; Ignore all logs, because there are lots of temporary warnings when developing and hot reloading @@ -45,10 +46,11 @@ :group-chats/extract-membership-signature :utils/dispatch-later :json-rpc/call}) - - (when (and js/goog.DEBUG platform/ios? DevSettings) - ;;on Android this method doesn't work - (when-let [nm (.-_nativeModule DevSettings)] - ;;there is a bug in RN, so we have to enable it first and then disable - (.setHotLoadingEnabled ^js nm true) - (js/setTimeout #(.setHotLoadingEnabled ^js nm false) 1000)))) + (when ^:boolean js/goog.DEBUG + (schema/setup!) + (when (and platform/ios? DevSettings) + ;;on Android this method doesn't work + (when-let [nm (.-_nativeModule DevSettings)] + ;;there is a bug in RN, so we have to enable it first and then disable + (.setHotLoadingEnabled ^js nm true) + (js/setTimeout #(.setHotLoadingEnabled ^js nm false) 1000))))) diff --git a/src/status_im2/setup/hot_reload.cljs b/src/status_im2/setup/hot_reload.cljs index be22d70a0b..38e83963b1 100644 --- a/src/status_im2/setup/hot_reload.cljs +++ b/src/status_im2/setup/hot_reload.cljs @@ -2,7 +2,9 @@ (:require [re-frame.core :as re-frame] [react-native.core :as rn] - [reagent.core :as reagent])) + [reagent.core :as reagent] + schema.state + [status-im2.setup.schema :as schema])) (defonce cnt (reagent/atom 0)) (defonce reload-locked? (atom false)) @@ -19,6 +21,8 @@ (reset! visible true) (reset! label "reloading UI") (re-frame/clear-subscription-cache!) + (schema/setup!) + (schema.state/clear-errors) (swap! cnt inc)) (defn before-reload diff --git a/src/status_im2/setup/schema.cljs b/src/status_im2/setup/schema.cljs new file mode 100644 index 0000000000..7a5ff28955 --- /dev/null +++ b/src/status_im2/setup/schema.cljs @@ -0,0 +1,104 @@ +(ns status-im2.setup.schema + (:require + [malli.core :as malli] + [malli.dev.cljs :as malli.dev] + [malli.dev.pretty :as malli.pretty] + [malli.dev.virhe :as malli.virhe] + malli.error + malli.instrument + malli.util + schema.common + [schema.core :as schema] + schema.registry + [taoensso.timbre :as log])) + +;;;; Formatters +;; These formatters replace the original ones provided by Malli. They are more +;; compact (less line breaks) and don't show the "More Information" section. + +(defn block + "Same as `malli.dev.pretty/-block`, but adds only one line break between `text` + and `body`." + [text body printer] + [:group (malli.virhe/-text text printer) :break [:align 2 body]]) + +(defmethod malli.virhe/-format ::malli/explain + [_ _ {:keys [schema] :as explanation} printer] + {:body + [:group + (block "Value:" (malli.virhe/-visit (malli.error/error-value explanation printer) printer) printer) + :break :break + (block "Errors:" (malli.virhe/-visit (malli.error/humanize explanation) printer) printer) + :break :break + (block "Schema:" (malli.virhe/-visit schema printer) printer)]}) + +(defmethod malli.virhe/-format ::malli/invalid-input + [_ _ {:keys [args input fn-name]} printer] + {:body + (cond-> [:group + (block "Invalid function arguments:" (malli.virhe/-visit args printer) printer) + :break :break] + fn-name + (conj (block "Function Var:" (malli.virhe/-visit fn-name printer) printer) + :break + :break) + :else + (conj (block "Input Schema:" (malli.virhe/-visit input printer) printer) + :break + :break + (block "Errors:" (malli.pretty/-explain input args printer) printer)))}) + +(defmethod malli.virhe/-format ::malli/invalid-output + [_ _ {:keys [value args output fn-name]} printer] + {:body + (cond-> [:group + (block "Invalid function return value:" (malli.virhe/-visit value printer) printer) + :break :break] + fn-name + (conj (block "Function Var:" (malli.virhe/-visit fn-name printer) printer) + :break + :break) + :else + (conj (block "Function arguments:" (malli.virhe/-visit args printer) printer) + :break + :break + (block "Output Schema:" (malli.virhe/-visit output printer) printer) + :break + :break + (block "Errors:" (malli.pretty/-explain output value printer) printer)))}) + +(defn register-schemas + "Register all global schemas in `schema.registry/registry`. + + Since keys in a map are unique, remember to qualify keywords. Prefer to add to + the global registry schemas for domain entities (e.g. message, chat, + notification, etc) or unambiguously useful schemas, like + `:schema.common/theme`." + [] + (schema.registry/merge (malli.util/schemas)) + (schema.common/register-schemas)) + +(defn setup! + "Configure Malli and initializes instrumentation. + + After evaluating an s-exp in the REPL that changes a function schema you'll + need to either save the file where the schema is defined and hot reload or + manually call `setup!`, otherwise you won't see any changes. It is safe and + even expected you will call `setup!` multiple times in REPLs." + [] + (try + (schema.registry/init-global-registry) + (register-schemas) + + ;; In theory not necessary, but sometimes in a REPL session the dev needs to + ;; call unstrument! manually. + (malli.instrument/unstrument!) + + (malli.dev/start! {:report (schema/reporter)}) + (println "Schemas initialized.") + + ;; It is relatively easy to write invalid schemas, but we don't want to + ;; block the app from initializing if such errors happen, at least not until + ;; Malli matures in the project. + (catch js/Error e + (log/error "Failed to initialize schemas" {:error e})))) diff --git a/src/status_im2/setup/schema_preload.cljs b/src/status_im2/setup/schema_preload.cljs new file mode 100644 index 0000000000..928ff0e587 --- /dev/null +++ b/src/status_im2/setup/schema_preload.cljs @@ -0,0 +1,6 @@ +(ns status-im2.setup.schema-preload + ":dev/always is needed so that the compiler doesn't cache this file." + {:dev/always true} + (:require [status-im2.setup.schema :as schema])) + +(schema/setup!) diff --git a/src/utils/money.cljs b/src/utils/money.cljs index 53cfd26b8f..68e5a07baa 100644 --- a/src/utils/money.cljs +++ b/src/utils/money.cljs @@ -2,6 +2,7 @@ (:require ["bignumber.js" :as BigNumber] [clojure.string :as string] + [schema.core :as schema] [utils.i18n :as i18n])) ;; The BigNumber version included in web3 sometimes hangs when dividing large @@ -237,3 +238,7 @@ :else (str amount)))) + +(schema/=> format-amount + [:=> [:cat [:maybe :int]] + [:maybe :string]]) diff --git a/test/jest/jest.config.js b/test/jest/jest.config.js index b6dc38e498..c0dce59644 100644 --- a/test/jest/jest.config.js +++ b/test/jest/jest.config.js @@ -1,6 +1,10 @@ module.exports = { preset: 'react-native', - setupFilesAfterEnv: ['@testing-library/jest-native/extend-expect', '../test/jest/jestSetup.js'], + setupFilesAfterEnv: [ + '@testing-library/jest-native/extend-expect', + '../component-spec/status_im2.setup.schema_preload.js', + '../test/jest/jestSetup.js', + ], setupFiles: [], testPathIgnorePatterns: [], moduleNameMapper: {