levand / quiescent

A lightweight ClojureScript abstraction over ReactJS
Eclipse Public License 1.0
613 stars 46 forks source link

No refresh of rendered HTML at all, when state is changed? #33

Closed ghost closed 9 years ago

ghost commented 10 years ago

Hi, I'm new to quiescent and ClojureScript in general. As an exercise for myself I'm making some adjustments to the quiescent TodoMVC app and slowly turn it into something else (rather than starting from scratch)

My latest change was the introduction of an HTML table, of which the contents should be automatically updated if the underlying state data is updated.

(q/defcomponent TableRow
  "Show a table row based on map containing row data"
  [row]
  (apply d/tr {} (map d/td (repeat {}) (vals row))))

(q/defcomponent Table
  "Create HTML table based on header text (vector) and table rows (vector of maps)"
  [header-columns table-rows]
  (d/table {}
   (d/thead {}
    (apply d/tr {} (map d/th (repeat {}) header-columns)))
   (apply d/tbody {} (map TableRow table-rows ))))

(q/defcomponent App
  "The root of the application"
  [{:keys [table-data]} channel]
  (d/div {}
         (Header nil channel)
         (d/section {} (Table ["Column A" "Column B" "Column C" "Column D"] table-data))))

;; JS console output of state after adding some rows with values 'a' 'b' 'c' 'd':
{:table-data [{:col-a "23-8-2014", :col-b "a", :col-c "b", :col-d "ba"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"} {:col-a "a", :col-b "b", :col-c "c", :col-d "d"}], :items [], :all-done? true} 

App is being called in the same way as is done in the standard TodoMVC app

Any change to :table-data is visible in the browsers Javascript output (so the state really changes), App is being called, but Table is not being called. What could be wrong? If you need more info, please let me know, and I'll provide.

samedhi commented 10 years ago

I am just skimming though this, but I have observed some errors when I don't include a <tbody> (which is automagically inserted betwee a <table> and a <tr> by the browser) in my React code. So try redefining Table with a d/tbody as well? Might help.

ghost commented 10 years ago

Hmm, but I'm already using tbody, see the line with:

   (apply d/tbody {} (map TableRow table-rows ))))
gaverhae commented 10 years ago

Quiescent only re-renders a component if its first argument has changed. In your case, the first argument (header-columns) is the same and only the second one (table-rows) is changed, so no re-render is triggered.

Simply swapping the arguments should work in this case. When designing a quiescent component, it is important to keep in mind that only the first argument will trigger re-renders, and hence all other arguments should only contain static data (such as your headers here).

This is documented in Static arguments in the doc:

To support this use case, Quiescent allows you to pass more than one argument to a component constructor. Any additional arguments to a Quiescent component constructor will be passed through as is to that component's rendering function, but will not be taken into account when deciding if a component needs to re-render.

Note that, before this section, all example uses of defcomponentin the doc take only a single argument and there is nothing to suggest that it is legal to pass more than one. Still, it might be worth moving that section a bit higher in the doc, or somehow putting more emphasis on this, as I understand it might be counter-intuitive.

ghost commented 10 years ago

@gaverhae thank you so much for pointing out that my argument vector was not correct. I've totally missed the static arguments part in the docs. I changed the argument order, and everything works as expected now.

I'll keep the issue open so @levand can decide if he wants to change the documentation, as you suggested. Otherwise, he can simply close this issue.

levand commented 9 years ago

I will keep this in mind next time I refresh the docs.

Thanks!