Merge pull request #301 from jeaye/assert-messages

Improve assert messages all around

CHANGELOG.md

@@ -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)
 

src/reagent/core.cljs

@@ -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]))
 
@@ -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
@@ -49,7 +52,7 @@
   "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
@@ -57,7 +60,7 @@
   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
@@ -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-component this)
-  (assert (or (nil? new-state) (map? new-state)))
+  (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-component this)
-  (assert (or (nil? new-state) (map? new-state)))
+  (assert-new-state new-state)
   (swap! (state-atom this) merge new-state))
 
 (defn force-update
@@ -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
@@ -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))
 
 

src/reagent/debug.clj

@@ -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))))

src/reagent/impl/batching.cljs

@@ -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]))
@@ -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)

src/reagent/impl/component.cljs

@@ -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*)
 
@@ -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)
@@ -201,8 +202,7 @@
 (defn get-wrapper [key f name]
   (let [wrap (custom-wrapper key f)]
     (when (and wrap f)
-      (assert (ifn? f)
+      (assert-callable f))
-              (str "Expected function in " name key " but got " f)))
     (or wrap f)))
 
 (def obligatory {:shouldComponentUpdate nil
@@ -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 "
+      (assert-callable render-fun)))
-                                     (pr-str render-fun)))))
   (let [render-fun (or (:reagentRender fmap)
                        (:componentFunction fmap))
         legacy-render (nil? render-fun)
@@ -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 "

src/reagent/impl/util.cljs

@@ -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))))))
 
 

src/reagent/interop.clj

@@ -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))

src/reagent/ratom.clj

@@ -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")

src/reagent/ratom.cljs

@@ -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)