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

Improve assert messages all around #301

Merged
merged 2 commits into from
Jun 27, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
methods to provide React, you need to exclude this dependency and provide it yourself.
- Self-host compatibility ([#283](https://github.com/reagent-project/reagent/pull/283))
- Removed deprecated `reagent.interop/.'` and `reagent.interop/.!` macros
- Improve assert messages all around

## 0.6.2 (19.5.2017)

Expand Down
35 changes: 19 additions & 16 deletions src/reagent/core.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
[reagent.impl.util :as util]
[reagent.impl.batching :as batch]
[reagent.ratom :as ratom]
[reagent.debug :as deb :refer-macros [dbg prn]]
[reagent.debug :as deb :refer-macros [dbg prn
assert-some assert-component
assert-js-object assert-new-state
assert-callable]]
[reagent.interop :refer-macros [$ $!]]
[reagent.dom :as dom]))

Expand All @@ -30,13 +33,13 @@
([type]
(create-element type nil))
([type props]
(assert (not (map? props)))
(assert-js-object props)
($ react createElement type props))
([type props child]
(assert (not (map? props)))
(assert-js-object props)
($ react createElement type props child))
([type props child & children]
(assert (not (map? props)))
(assert-js-object props)
(apply ($ react :createElement) type props child children)))

(defn as-element
Expand All @@ -49,15 +52,15 @@
"Returns an adapter for a native React class, that may be used
just like a Reagent component function or class in Hiccup forms."
[c]
(assert c)
(assert-some c "Component")
(tmpl/adapt-react-class c))

(defn reactify-component
"Returns an adapter for a Reagent component, that may be used from
React, for example in JSX. A single argument, props, is passed to
the component, converted to a map."
[c]
(assert c)
(assert-some c "Component")
(comp/reactify-component c))

(defn render
Expand Down Expand Up @@ -125,30 +128,30 @@
(defn state-atom
"Returns an atom containing a components state."
[this]
(assert (comp/reagent-component? this))
(assert-component this)
(comp/state-atom this))

(defn state
"Returns the state of a component, as set with replace-state or set-state.
Equivalent to (deref (r/state-atom this))"
[this]
(assert (comp/reagent-component? this))
(assert-component this)
(deref (state-atom this)))

(defn replace-state
"Set state of a component.
Equivalent to (reset! (state-atom this) new-state)"
[this new-state]
(assert (comp/reagent-component? this))
(assert (or (nil? new-state) (map? new-state)))
(assert-component this)
(assert-new-state new-state)
(reset! (state-atom this) new-state))

(defn set-state
"Merge component state with new-state.
Equivalent to (swap! (state-atom this) merge new-state)"
[this new-state]
(assert (comp/reagent-component? this))
(assert (or (nil? new-state) (map? new-state)))
(assert-component this)
(assert-new-state new-state)
(swap! (state-atom this) merge new-state))

(defn force-update
Expand All @@ -166,19 +169,19 @@
(defn props
"Returns the props passed to a component."
[this]
(assert (comp/reagent-component? this))
(assert-component this)
(comp/get-props this))

(defn children
"Returns the children passed to a component."
[this]
(assert (comp/reagent-component? this))
(assert-component this)
(comp/get-children this))

(defn argv
"Returns the entire Hiccup form passed to the component."
[this]
(assert (comp/reagent-component? this))
(assert-component this)
(comp/get-argv this))

(defn dom-node
Expand Down Expand Up @@ -257,7 +260,7 @@

Probably useful only for passing to child components."
[value reset-fn & args]
(assert (ifn? reset-fn))
(assert-callable reset-fn)
(ratom/make-wrapper value reset-fn args))


Expand Down
23 changes: 23 additions & 0 deletions src/reagent/debug.clj
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,26 @@ as well as package name and line number. Returns x."
~@forms)]
(js/console.timeEnd label#)
res#)))

(defmacro assert-some [value tag]
`(assert ~value (str ~tag " must not be nil")))

(defmacro assert-component [value]
`(assert (comp/reagent-component? ~value)
(str "Expected a reagent component, not "
(pr-str ~value))))

(defmacro assert-js-object [value]
`(assert (not (map? ~value))
(str "Expected a JS object, not "
(pr-str ~value))))

(defmacro assert-new-state [value]
`(assert (or (nil? ~value) (map? ~value))
(str "Expected a valid new state, not "
(pr-str ~value))))

(defmacro assert-callable [value]
`(assert (ifn? ~value)
(str "Expected something callable, not "
(pr-str ~value))))
4 changes: 2 additions & 2 deletions src/reagent/impl/batching.cljs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
(ns reagent.impl.batching
(:refer-clojure :exclude [flush])
(:require [reagent.debug :refer-macros [dbg]]
(:require [reagent.debug :refer-macros [dbg assert-some]]
[reagent.interop :refer-macros [$ $!]]
[reagent.impl.util :refer [is-client]]
[clojure.string :as string]))
Expand Down Expand Up @@ -45,7 +45,7 @@
(deftype RenderQueue [^:mutable ^boolean scheduled?]
Object
(enqueue [this k f]
(assert (some? f))
(assert-some f "Enqueued function")
(when (nil? (aget this k))
(aset this k (array)))
(.push (aget this k) f)
Expand Down
13 changes: 6 additions & 7 deletions src/reagent/impl/component.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
[reagent.impl.batching :as batch]
[reagent.ratom :as ratom]
[reagent.interop :refer-macros [$ $!]]
[reagent.debug :refer-macros [dbg prn dev? warn error warn-unless]]))
[reagent.debug :refer-macros [dbg prn dev? warn error warn-unless
assert-callable]]))

(declare ^:dynamic *current-component*)

Expand Down Expand Up @@ -85,7 +86,7 @@

(defn wrap-render [c]
(let [f ($ c :reagentRender)
_ (assert (ifn? f))
_ (assert-callable f)
res (if (true? ($ c :cljsLegacyRender))
(.call f c c)
(let [v (get-argv c)
Expand Down Expand Up @@ -201,8 +202,7 @@
(defn get-wrapper [key f name]
(let [wrap (custom-wrapper key f)]
(when (and wrap f)
(assert (ifn? f)
(str "Expected function in " name key " but got " f)))
(assert-callable f))
(or wrap f)))

(def obligatory {:shouldComponentUpdate nil
Expand All @@ -225,8 +225,7 @@
render-fun (-> renders vals first)]
(assert (pos? (count renders)) "Missing reagent-render")
(assert (== 1 (count renders)) "Too many render functions supplied")
(assert (ifn? render-fun) (str "Render must be a function, not "
(pr-str render-fun)))))
(assert-callable render-fun)))
(let [render-fun (or (:reagentRender fmap)
(:componentFunction fmap))
legacy-render (nil? render-fun)
Expand Down Expand Up @@ -291,7 +290,7 @@
""))

(defn fn-to-class [f]
(assert (ifn? f) (str "Expected a function, not " (pr-str f)))
(assert-callable f)
(warn-unless (not (and (react-class? f)
(not (reagent-class? f))))
"Using native React classes directly in Hiccup forms "
Expand Down
3 changes: 2 additions & 1 deletion src/reagent/impl/util.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@
(if (nil? p1)
p2
(do
(assert (map? p1))
(assert (map? p1)
(str "Property must be a map, not " (pr-str p1)))
(merge-style p1 (merge-class p1 (merge p1 p2))))))


Expand Down
2 changes: 1 addition & 1 deletion src/reagent/interop.clj
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
(defn- dot-args [object member]
(assert (or (symbol? member)
(keyword? member))
(str "Symbol or keyword expected, not " member))
(str "Symbol or keyword expected, not " (pr-str member)))
(assert (or (not (symbol? object))
(not (re-find #"\." (name object))))
(str "Dot not allowed in " object))
Expand Down
4 changes: 3 additions & 1 deletion src/reagent/ratom.clj
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
co#))

(defmacro with-let [bindings & body]
(assert (vector? bindings))
(assert (vector? bindings)
(str "with-let bindings must be a vector, not "
(pr-str bindings)))
(let [v (gensym "with-let")
k (keyword v)
init (gensym "init")
Expand Down
2 changes: 1 addition & 1 deletion src/reagent/ratom.cljs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@

IReset
(-reset! [a newval]
(assert (fn? (.-on-set a)) "Reaction is read only.")
(assert (fn? (.-on-set a)) "Reaction is read only; on-set is not allowed")
(let [oldval state]
(set! state newval)
(.on-set a oldval newval)
Expand Down