reagent-project / reagent

A minimalistic ClojureScript interface to React.js
http://reagent-project.github.io/
MIT License
4.74k stars 412 forks source link

Better error messages on Invalid Hiccup form #382

Open frerom opened 6 years ago

frerom commented 6 years ago

Getting an exception like Assert failed: Invalid Hiccup form: [nil.. can be hard to investigate since the function generating the hiccup isn't in the stack trace. It would be nice to somehow get a better error message for this.

mike-thompson-day8 commented 6 years ago

For anyone to action this, you'll need to be more specific.

Please supply an example of the broken hiccup (perhaps a Form-1 component which returns some bad hiccup) and the resulting poor error message which needs to be improved.

cbillowes commented 2 years ago

@mike-thompson-day8 I just got this and thought this information may help somehow.

I can't supply the bad hiccup because I am not sure why the hiccup is bad in this first place 😞 The hiccup for the component being rendered is most likely nil. I'm referencing a third party component (in this case [c/click]) whose implementation is hidden behind a JavaScript npm package.

The error and stack trace are as follows:

template.cljs:271 Uncaught Error: Assert failed: Invalid Hiccup form: [nil]
 (in shadow_ninja.components.core.click)
(valid-tag? tag)
    at Object.reagent$impl$template$vec_to_elem [as vec_to_elem] (template.cljs:271)
    at Object.reagent$impl$template$as_element [as as_element] (template.cljs:288)
    at Object.eval [as reagent$impl$protocols$Compiler$as_element$arity$2] (template.cljs:305)
    at Object.reagent$impl$protocols$as_element [as as_element] (protocols.cljs:5)
    at Object.reagent$impl$component$wrap_render [as wrap_render] (component.cljs:93)
    at Object.reagent$impl$component$do_render [as do_render] (component.cljs:121)
    at eval (component.cljs:271)
    at Object.reagent$ratom$in_context [as in_context] (ratom.cljs:44)
    at Object.reagent$ratom$deref_capture [as deref_capture] (ratom.cljs:57)
    at Object.reagent$ratom$run_in_reaction [as run_in_reaction] (ratom.cljs:539)

Based on the stack trace the assertion specifies that the error occurred in a specific file: (in shadow_ninja.components.core.click) so this issue seems to have been addressed.

Deraen commented 2 years ago

@cbillowes Is this with Reagent 1.1.0? https://github.com/reagent-project/reagent/commit/edd3c59c86c921bc5bf92abcf86f7fa215dde11e might have helped to clear up the stacktrace.

There really isn't much we can do to improve error reporting more for cases where the component is nil. Because Reagent interprets the hiccup-style forms in runtime, we don't see the original symbol (e.g. c/click) at all. We only see the value, if the symbol points to JS object (React library etc.) and there is something wrong with the require form or something, we don't know anything where the value came from.

cbillowes commented 2 years ago

@Deraen I am using Reagent 1.1.0.

I agree that there isn't much that can be done regarding improving error reporting for cases where the component is nil.

Based on the original issue Getting an exception like Assert failed: Invalid Hiccup form: [nil.. can be hard to investigate since the function generating the hiccup isn't in the stack trace. It would be nice to somehow get a better error message for this. I wanted to show that the stack trace I get shows me which component the error assertion originated from. Based on that a conclusion can be made to either keep the issue open or mark it as resolved. That is assuming the person having the issue experienced the same experience as me.

Deraen commented 2 years ago

Yeah I'm thinking about closing this.

One case that might hide the function where hiccup is generated, is form-2 or 3 case, where the render fn doesn't have a name:

(defn foo []
  (let [asd (atom nil)]
    (fn []
       [c/click ...])))

The render fn doesn't have a name here.

It might be possible to add name to render fns based on the top-level fn name, for development builds. Function-render impl does similar tricks.

frerom commented 2 years ago

No reason why you couldn't name the inner function either!