Replace component stack with just component name in error messages

- Completely remove component-path function
- Remove component stack information from error messages
- Test that component stack information is availble using Error Boundary

CHANGELOG.md

@@ -4,13 +4,15 @@
 
 - Removed deprecated namespaces/function/macros:
     - Removed `reagent.interop` namespace (macros `$`, `$!`, `unchecked-aget` and `unchecked-aset`)
-- Deprecated functions:
     - `reagent.core/component-path` (Reason: implementation depends on internal React
     details and using just Component `displayName` achieves nearly the same)
 - All DOM related functions (notably `render` and `dom-node`) have been removed
 from `reagent.core` namespace and are now only available in `reagent.dom` namespace.
 This is to make non-DOM environments (React-native) first class targets with Reagent,
 as requiring `react-dom` always causes problems in such environments.
+- `Error rendering component` messages no longer contain component stack information.
+Instead one should use [an Error Boundary](https://reactjs.org/docs/error-boundaries.html#component-stack-traces)
+to catch the problem and look at the error information `componentStack` property.
 
 ## 0.9.1 (2020-01-15)
 

src/reagent/core.cljs

@@ -345,11 +345,3 @@
   "Works just like clojure.core/partial, but the result can be compared with ="
   [f & args]
   (util/make-partial-fn f args))
-
-(defn component-path
-  "Try to return the path of component c as a string.
-  Maybe useful for debugging and error reporting, but may break
-  with future versions of React (and return nil)."
-  {:deprecated "1.0.0"}
-  [c]
-  (comp/component-path c))

src/reagent/impl/component.cljs

@@ -120,7 +120,17 @@
                    (recur c))
       :else res)))
 
-(declare comp-name)
+(defn component-name [c]
+  (some-> c .-constructor .-displayName))
+
+(defn comp-name []
+  (if (dev?)
+    (let [c *current-component*
+          n (component-name c)]
+      (if-not (empty? n)
+        (str " (in " n ")")
+        ""))
+    ""))
 
 (defn do-render [c]
   (binding [*current-component* c]
@@ -362,50 +372,6 @@
 
     cmp))
 
-(defn fiber-component-path [fiber]
-  (let [name (some-> fiber
-                     (.-type)
-                     (.-displayName))
-        parent (some-> fiber
-                       (.-return))
-        path (some-> parent
-                     fiber-component-path
-                     (str " > "))
-        res (str path name)]
-    (when-not (empty? res) res)))
-
-(defn component-path [c]
-  ;; Alternative branch for React 16
-  ;; Try both original name (for UMD foreign-lib) and manged name (property access, for Closure optimized React)
-  (if-let [fiber (or (some-> c (gobj/get "_reactInternalFiber"))
-                     (some-> c (.-_reactInternalFiber)))]
-    (fiber-component-path fiber)
-    (let [instance (or (some-> c (gobj/get "_reactInternalInstance"))
-                       (some-> c (.-_reactInternalInstance))
-                       c)
-          elem (or (some-> instance (gobj/get "_currentElement"))
-                   (some-> instance (.-_currentElement)))
-          name (some-> elem
-                       (.-type)
-                       (.-displayName))
-          owner (or (some-> elem (gobj/get "_owner"))
-                    (some-> elem (.-_owner)))
-          path (some-> owner
-                       component-path
-                       (str " > "))
-          res (str path name)]
-      (when-not (empty? res) res))))
-
-(defn comp-name []
-  (if (dev?)
-    (let [c *current-component*
-          n (or (component-path c)
-                (some-> c .-constructor util/fun-name))]
-      (if-not (empty? n)
-        (str " (in " n ")")
-        ""))
-    ""))
-
 (defn fn-to-class [f]
   (assert-callable f)
   (warn-unless (not (and (react-class? f)

test/reagenttest/testreagent.cljs

@@ -563,7 +563,7 @@
                 (wrap-capture-console-error
                   #(with-mounted-component [c]
                      (fn [c div]))))]
-        (if (debug/dev?)
+        (if (dev?)
           (is (= "Warning: Every element in a seq should have a unique :key: ([:button {:on-click #object[Function]}] [:button {:on-click #object[Function]}] [:button {:on-click #object[Function]}])\n (in reagenttest.testreagent.key_tester)"
                  (first (:warn w)))))))))
 
@@ -664,7 +664,7 @@
         tc (r/create-class {:display-name "atestcomponent"
                             :render (fn []
                                       (let [c (r/current-component)]
-                                        (reset! a (comp/component-path c))
+                                        (reset! a (comp/component-name c))
                                         [:div]))})]
     (with-mounted-component [tc]
       (fn [c]
@@ -1013,18 +1013,19 @@
                   cmp)
             pkg "reagenttest.testreagent."
             stack1 (str "in " pkg "comp1")
-            stack2 (str "in " pkg "comp2 > " pkg "comp1")
             re (fn [& s]
                  (re-pattern (apply str s)))
             rend (fn [x]
                    (with-mounted-component x identity))]
+
+        ;; Error is orginally caused by comp1, so only that is shown in the error
         (let [e (debug/track-warnings
                   (wrap-capture-window-error
                     (wrap-capture-console-error
                       #(is (thrown-with-msg?
-                             :default (re "Invalid tag: 'div.' \\(" stack2 "\\)")
+                             :default (re "Invalid tag: 'div.' \\(" stack1 "\\)")
                              (rend [comp2 [:div. "foo"]]))))))]
-          (is (= (str "Error rendering component (" stack2 ")")
+          (is (= (str "Error rendering component (" stack1 ")")
                  (last (:error e)))))
 
         (let [e (debug/track-warnings
@@ -1052,9 +1053,11 @@
 
 (deftest test-error-boundary
   (let [error (r/atom nil)
+        info (r/atom nil)
         error-boundary (fn error-boundary [comp]
                          (r/create-class
-                           {:component-did-catch (fn [this e info])
+                           {:component-did-catch (fn [this e i]
+                                                   (reset! info i))
                             :get-derived-state-from-error (fn [e]
                                                             (reset! error e)
                                                             #js {})
@@ -1063,15 +1066,22 @@
                                                 [:div "Something went wrong."]
                                                 comp))}))
         comp1 (fn comp1 []
-                (throw (js/Error. "Test error")))]
+                (throw (js/Error. "Test error")))
+        comp2 (fn comp2 []
+                [comp1])]
     (debug/track-warnings
       (wrap-capture-window-error
         (wrap-capture-console-error
-          #(with-mounted-component [error-boundary [comp1]]
+          #(with-mounted-component [error-boundary [comp2]]
              (fn [c div]
                (r/flush)
                (is (= "Test error" (.-message @error)))
-               (is (re-find #"Something went wrong\." (.-innerHTML div))))))))))
+               (is (re-find #"Something went wrong\." (.-innerHTML div)))
+               (if (dev?)
+                 (is (re-find #"\n    in reagenttest.testreagent.comp1 \(created by reagenttest.testreagent.comp2\)\n    in reagenttest.testreagent.comp2 \(created by reagent[0-9]+\)\n    in reagent[0-9]+ \(created by reagenttest.testreagent.error_boundary\)\n    in reagenttest.testreagent.error_boundary"
+                       (.-componentStack @info)))
+                 (is (re-find #"\n    in .+\n    in .+\n    in reagent[0-9]+\n    in .+"
+                       (.-componentStack @info))) ))))))))
 
 (deftest test-dom-node
   (let [node (atom nil)