reagent-project / reagent

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

Nesting components documentation #68

Closed lorddoig closed 9 years ago

lorddoig commented 9 years ago

With regard to #13, I see from the discussion that the problem itself appears fixed, but I'm having a little trouble understanding exactly what the fix was. Currently, I'm just doing

(defn foo
  [props & children]
  [:div
    children])

and ignoring the react warning about child arrays and keys.

I'm assuming #13 sorted this? If so, some docs would be great. Even if someone can explain to me briefly here how this is now done properly I'll write and PR the docs myself.

mike-thompson-day8 commented 9 years ago

@lorddoig The discussion in #13 lead to the current design, which is what you'll find described in the existing README and other docs.

Perhaps it will help if I give you some working code related to the initial question posed in #13.

The following code works just fine with reagent these days:

(defn my-li
  [a-string]
  [:li
    ;; {:style {:color a-string}}     ;; optional props
    a-string])

(defn my-list
   [& children]
   [:ul 
      ;; {:style {:background-color "#CCC"}}     ;; optional props
      children])

(reagent/render-component 
    [my-list
      [my-li "green"]
      [my-li "blue"]]
    (. js/document (getElementById "app")))

Perhaps this very similar variation might help to tease out the issues for you:


(defn my-li
  [name as-hex]                  ;; two of them
  [:li
    {:style {:color as-hex}}    
    name])

(defn my-list
   [background & children]
   [:ul 
      {:style {:background-color background}}   
      children])

(reagent/render-component 
    [my-list
      "#CCC"                        ;; give some value BEFORE the children
      [my-li "green" "#0A0"]        ;; give two "parameters":  name and hex value
      [my-li "blue" "#00A"]]
    (. js/document (getElementById "app")))

But apart from that, just read the existing README and other docs.

mike-thompson-day8 commented 9 years ago

I think we can close this

lorddoig commented 9 years ago

@mike-thompson-day8 Thank you for the example. As you point out, it works fine - I'm not sure what was going on before. Apologies for the wasted time.

lorddoig commented 9 years ago

@mike-thompson-day8 So I'm back to having issues with this. It works, yes, but whereas before I was getting these enormous warnings from React, I'm now getting enormous warnings from Reagent itself (and when I made my last comment I'm 99% sure there were no warnings at all.) Using your exact example above:

(from template.cljs:279)

Warning: Every element in a seq should have a unique :key (in reagent1). Value: ([#<function my_li(a_string){
return new cljs.core.PersistentVector(null, 2, 5, cljs.core.PersistentVector.EMPTY_NODE, [new cljs.core.Keyword(null,"li","li",723558921),a_string], null);
}> "one"] [#<function my_li(a_string){
return new cljs.core.PersistentVector(null, 2, 5, cljs.core.PersistentVector.EMPTY_NODE, [new cljs.core.Keyword(null,"li","li",723558921),a_string], null);
}> "two"])

Like I say it works fine but I'm assuming the warning exists because React needs these keys for performance? And surely a library's "official" way of doing something shouldn't then cause said library to generate warnings? (Edit: and the main reason is it pretty much takes over my console output!)

I went back to have a look and neither the README nor the demo page touch on the [& children] thing. It's the natural way to go about it so people new to Reagent will do that, see these warnings, go back to readme/demo, find nothing, assume they're doing it wrong, and not be able to figure out the right way - which is exactly what led to the opening of this issue in the first place.

In case I'm being a complete idiot here, the exact code I'm using to test this:

(defn my-li
  [a-string]
  [:li
   ;; {:style {:color a-string}}     ;; optional props
   a-string])

(defn my-list
  [& children]
  [:ul
   ;; {:style {:background-color "#CCC"}}     ;; optional props
   children])

(defn thelist []
  [my-list
   [my-li "one"]
   [my-li "two"]])

  ; One of the following, tried them all to be 100% sure
  (reagent/render-component (thelist) (.getElementById js/document "app"))
  ; (reagent/render-component thelist (.getElementById js/document "app"))
  ; (reagent/render-component [thelist] (.getElementById js/document "app"))
  ; (reagent/render-component [my-list [my-li "one"] [my-li "two"]] (.getElementById js/document "app"))

Using the latest stable versions of reagent, react, and clojurescript.

I'm sorry to drag this out but what is the correct way to do this?

mike-thompson-day8 commented 9 years ago

87 improved key-less-ness reporting. Previously, if you got a no-key-provided warning from React, it was hard to figure out where it was coming from.

Although useful, the new reagent warning introduced in #87 is pretty massive and can be quite confusing.

@holmsand can I suggest that the "Value" part of the message (which can be huge) should be hidden inside a grouped output.

So this code might become more like this javasript pseduo code: console.groupCollapsed("Every element in a seq should have a unique key") console.log("Key" xxxx); console.log("Value" xxxx); console.groupEnd();

See console reference

@lorddoig this tweak should fix the problem for you (untested):

(defn my-list
  [& children]
  [:ul
   (map-indexed #(with-meta %2  {:key %1}) children)])    ;; add  key metadata to each child
pragyanatvade commented 5 years ago

@mike-thompson-day8

I am a beginner in ClojureScript and Re-frame I am trying to do something as follows:

(ns vds.atoms.grid
  (:require 
   [stylefy.core :refer [use-style]]
   [vds.utils :as vdu]
   [vds.atoms.styles.grid :as styles]))

(defn vda-grid
  [& args]
  (let [[{:keys [style]}]
        (vdu/extract-args args)]
    (fn [& args]
      (let [[_ children]
            (vdu/extract-args args)]
        [:div 
         (use-style (merge (styles/vda-grid {:style style})))
         (map-indexed #(with-meta %2  {:key %1}) children)]))))

(defn app
  []
  [vda-grid
   (for [x (range 1 20)]
     [:div (str "BOX " x)])])

But I still see the following warning in console:

Warning: Every element in a seq should have a unique :key: ([:div "BOX 1"] [:div "BOX 2"] [:div "BOX 3"] [:div "BOX 4"] [:div "BOX 5"] [:div "BOX 6"] [:div "BOX 7"] [:div "BOX 8"] [:div "BOX 9"] [:div "BOX 10"] [:div "BOX 11"] [:div "BOX 12"] [:div "BOX 13"] [:div "BOX 14"] [:div "BOX 15"] [:div "BOX 16"] [:div "BOX 17"] [:div "BOX 18"] [:div "BOX 19"])
 (in vds.atoms.grid.vda_grid)

If I use the app method as follows it works

(defn app
  []
  [vda-grid
   (for [x (range 1 20)]
     [:div {:key x} (str "BOX " x)])])

I am trying to understand why your solution is not working in this case or am I doing anything wrong?

mike-thompson-day8 commented 5 years ago

@pntripathi9417 Reagent Issues is a place for reporting errors or requesting features. It is not for support.

You should instead join clojurians slack at https://clojurians.herokuapp.com/ and then ask for help in the #reagent channel.

pragyanatvade commented 5 years ago

@mike-thompson-day8 Sure thing. I thought it might be an issue after going through the whole conversation. Will get help from the slack channel. Thanks.