From 7df2941c7d5036d70b7db5b8d26867840036de56 Mon Sep 17 00:00:00 2001 From: Dan Holmsand Date: Mon, 2 Feb 2015 13:41:29 +0100 Subject: [PATCH] Better error reporting Warnings are now printed using console.warn. The name of the current component is included in more cases. Also warn every time an atom is derefed in a lazy seq, and not just the first. --- src/reagent/debug.clj | 13 +++++++++++++ src/reagent/impl/component.cljs | 28 ++++++++++++++++++++-------- src/reagent/impl/template.cljs | 30 ++++++++++++++++-------------- src/reagent/impl/util.cljs | 4 ++-- src/reagent/ratom.cljs | 13 ++++++------- 5 files changed, 57 insertions(+), 31 deletions(-) diff --git a/src/reagent/debug.clj b/src/reagent/debug.clj index c2a044c..2e673bd 100644 --- a/src/reagent/debug.clj +++ b/src/reagent/debug.clj @@ -8,6 +8,19 @@ `(when (clojure.core/exists? js/console) (.log js/console ~@forms))) +(defmacro warn + "Print with console.warn." + [& forms] + (when *assert* + `(when (clojure.core/exists? js/console) + (.warn js/console (str "Warning: " ~@forms))))) + +(defmacro warn-unless + [cond & forms] + (when *assert* + `(when (and (not ~cond) (clojure.core/exists? js/console)) + (.warn js/console (str "Warning: " ~@forms))))) + (defmacro println "Print string with console.log" [& forms] diff --git a/src/reagent/impl/component.cljs b/src/reagent/impl/component.cljs index c3297f5..19d0f50 100644 --- a/src/reagent/impl/component.cljs +++ b/src/reagent/impl/component.cljs @@ -3,7 +3,7 @@ [reagent.impl.batching :as batch] [reagent.ratom :as ratom] [reagent.interop :refer-macros [.' .!]] - [reagent.debug :refer-macros [dbg prn]])) + [reagent.debug :refer-macros [dbg prn dev?]])) (declare ^:dynamic *current-component*) @@ -118,10 +118,10 @@ (this-as c (apply f c args))) f)) -(def dont-wrap #{:cljsRender :render :componentFunction}) +(def dont-wrap #{:cljsRender :render :componentFunction :cljsName}) (defn dont-bind [f] - (if (ifn? f) + (if (fn? f) (doto f (.! :__reactDontBind true)) f)) @@ -149,10 +149,13 @@ (defn add-obligatory [fun-map] (merge obligatory fun-map)) -(defn add-render [fun-map render-f] - (assoc fun-map - :cljsRender render-f - :render (:render static-fns))) +(defn add-render [fun-map render-f name] + (let [fm (assoc fun-map + :cljsRender render-f + :render (:render static-fns))] + (if (dev?) + (assoc fm :cljsName (fn [] name)) + fm))) (defn wrap-funs [fun-map] (let [render-fun (or (:componentFunction fun-map) @@ -167,7 +170,7 @@ name' (if (empty? name) (str (gensym "reagent")) name) fmap (-> fun-map (assoc :displayName name') - (add-render render-fun))] + (add-render render-fun name'))] (reduce-kv (fn [m k v] (assoc m k (get-wrapper k v name'))) {} fmap))) @@ -195,3 +198,12 @@ (util/cache-react-class f res) (util/cache-react-class res res) f)) + +(defn comp-name [] + (if (dev?) + (let [n (some-> *current-component* + (.' cljsName))] + (if-not (empty? n) + (str " (in " n ")") + "")) + "")) diff --git a/src/reagent/impl/template.cljs b/src/reagent/impl/template.cljs index eb9d292..9e057b9 100644 --- a/src/reagent/impl/template.cljs +++ b/src/reagent/impl/template.cljs @@ -5,8 +5,8 @@ [reagent.impl.batching :as batch] [reagent.ratom :as ratom] [reagent.interop :refer-macros [.' .!]] - [reagent.debug :refer-macros [dbg prn println log dev?]])) - + [reagent.debug :refer-macros [dbg prn println log dev? + warn warn-unless]])) ;; From Weavejester's Hiccup, via pump: (def ^{:doc "Regular expression that parses a CSS-style id and class @@ -149,18 +149,18 @@ (let [[tag id class] (->> hiccup-tag name (re-matches re-tag) next) class' (when class (string/replace class #"\." " "))] - (assert tag (str "Unknown tag: '" hiccup-tag "'")) + (assert tag (str "Invalid tag: '" hiccup-tag "'" + (comp/comp-name))) #js{:name tag :id id :className class'})) (defn fn-to-class [f] (assert (ifn? f) (str "Expected a function, not " (pr-str f))) - (when (dev?) - (when (and (fn? f) - (some? (.' f :type))) - (log "warning: using native React classes directly is not supported: " - (.' f :type)))) + (warn-unless (not (and (fn? f) + (some? (.' f :type)))) + "Using native React classes directly " + "is not supported: " (.' f :type) (comp/comp-name)) (let [spec (meta f) withrender (assoc spec :component-function f) res (comp/create-class withrender) @@ -215,10 +215,13 @@ (make-element argv comp p first-child))))))) (defn vec-to-elem [v] - (assert (pos? (count v)) "Hiccup form should not be empty") + (assert (pos? (count v)) + (str "Hiccup form should not be empty: " + (pr-str v) (comp/comp-name))) (let [tag (nth v 0)] (assert (valid-tag? tag) - (str "Invalid Hiccup form: " (pr-str v))) + (str "Invalid Hiccup form: " + (pr-str v) (comp/comp-name))) (if-some [ne (native-element tag v)] ne (reag-element tag v)))) @@ -226,10 +229,9 @@ (def seq-ctx #js{}) (defn warn-on-deref [x] - (when-not (.' seq-ctx :warned) - (log "Warning: Reactive deref not supported in seq in " - (pr-str x)) - (.! seq-ctx :warned true))) + (warn "Reactive deref not supported in lazy seq, it should be " + "wrapped in doall" (comp/comp-name) ". Value:\n" + (pr-str x))) (declare expand-seq) diff --git a/src/reagent/impl/util.cljs b/src/reagent/impl/util.cljs index b421ffe..ce62a24 100644 --- a/src/reagent/impl/util.cljs +++ b/src/reagent/impl/util.cljs @@ -1,5 +1,5 @@ (ns reagent.impl.util - (:require [reagent.debug :refer-macros [dbg log]] + (:require [reagent.debug :refer-macros [dbg log warn]] [reagent.interop :refer-macros [.' .!]] [clojure.string :as string])) @@ -110,7 +110,7 @@ (try (.' js/React unmountComponentAtNode node) (catch js/Object e - (do (log "Error unmounting:") + (do (warn "Error unmounting:") (log e))))) (defn render-component [comp container callback] diff --git a/src/reagent/ratom.cljs b/src/reagent/ratom.cljs index edebf05..a06ab22 100644 --- a/src/reagent/ratom.cljs +++ b/src/reagent/ratom.cljs @@ -1,6 +1,6 @@ (ns reagent.ratom (:refer-clojure :exclude [atom]) - (:require-macros [reagent.debug :refer (dbg log dev?)]) + (:require-macros [reagent.debug :refer (dbg log warn dev?)]) (:require [reagent.impl.util :as util])) (declare ^:dynamic *ratom-context*) @@ -162,10 +162,9 @@ [src path] (if (satisfies? IDeref path) (do - (when (dev?) - (log (str "Calling cursor with an atom as the second arg is " - "deprecated, in (cursor " - src " " (pr-str path) ")"))) + (warn "Calling cursor with an atom as the second arg is " + "deprecated, in (cursor " + src " " (pr-str path) ")") (RCursor. path src nil)) (do (assert (or (satisfies? IDeref src) @@ -325,8 +324,8 @@ (-deref [this] (when (dev?) (when (and changed (some? *ratom-context*)) - (log "warning: derefing stale wrap: " - (pr-str this)))) + (warn "derefing stale wrap: " + (pr-str this)))) state) IReset