Hi, not sure if this is the best place to ask about sablono behaviours so I apologise beforehand. I want to ask about the behaviour of list in sabolono and how to return multiple elements from a form (for example a conditional)

[:div
   [:span "a"]
   [:span "b"]]

valid, no errors

----
[:div
   [:span "a"]
  (list [:span "b"]])

react, unique key error 'Each child in an array or iterator should have a unique "key" prop'

fixed by doing 

[:div
   [:span "a"]
  (list [:span {:key "b"} "b"]])

Happens if i swap the span within the list to a diff element (such as p or div).

Is there some rule where all elements returned from list must have a unique key, or be lists themselves? That doesn't seem right because the following doesn't error.

[:div
   [:span "a"]
   (list [:span {:key "b"} "b"] (list [:span "x"]))]

or even

[:div
   [:span "a"]
   (list [:span {:key "b"} "b"] (list [:span "x"] [:span "y"]))]

@mbutler sequences are seen as similar items of the same kind and so they do need a key for effective reconcilaition inside the DOM tree

@mbutler you can trick react into not seeing this if you use (into [:div] (list [:span "a"] [:span "b"])) but generally it’s not a bad idea to provide react-keys when you render many things of the same kind

Okay, that makes sense @martinklepsch, so its sabolono eagerly assuming that what i return from my list are similar elements even if they are not?

[:div
   [:span "1"]
   (list [:span "a"] [:div "b"] [:p "c"]) ]

Error 

----

[:div
   [:span "1"]
   (list [:span {:key "a"} "a"] [:div {:key "b"} "b"] [:p {:key "c"} "c"]) ]

No error

it’s not sablono, it’s react

if react sees a list it assumes they are similar items, and thus need a key

But in reality doesnt actually need it? Or as far as reacts concerned they ARE similar because they are in a list?

if you only have 3-10 items in the list the need might be debatable depending on the items own complexity

but yes, as far as react is concerned items in a list are similar and thus should have a key

I think i might be missusing list then. All i wished to do was return multiple items from a conditional rather than repeating the conditional, they are in no way actually a list of things.

[:div
   [:span "1"]
   (when test (list [:span "a"] [:div "b"] [:p "c"]))] 

(into [:div]
      (if false
        [[:span “foo”]]
        [[:span “bar”]
         [:span “xxx”]]))

So you think its ok to trick react if i know what im doing is not returning a "list" ofthings

sure

I think the “this needs a key” warnings are intended as guidance not hard rule, it’s a machine interpreting your code after all, can’t know your intent

I think thats a very "fair" statement. I have a followup question thats slightly related, now I think i understand what list is doing

(map #(list [:br] [:span %]) ["a" "b" "c"])

React doesnt want keys when i do something like this

it seems like that should fall under the same rule as the previous one.

@martinklepsch

is it just an oversight? when you return interleaved stuff like this does react want keys it just doesnt know it?

@rauh if you don’t mind, could you check if I did everything as you suggested here: https://github.com/tonsky/rum/pull/145/files ? I get DCE to work but only if I remove the specify! call

@martinklepsch Yeah I'm looking at it as we speak

@rauh nice, thank you very much and sorry for bugging 🙂

@martinklepsch Well shoot, I must've tested it incorrectly, GCC probably inlined the string so I didn't find it and thought it was DCE'd.

It looks like the only way to do this is to create a custom Datatype just like MetaFn and realize the meta data lazily

So (LazyMetaFn. f c) which then returns the meta of (c) in IMeta

Right. So given that this whole rum/class metadata thing isn’t documented and seems generally troubling wrt DCE — maybe it should be considered for removal?

Yeah just put in an API, thats why we have API's so we can change the underlying implementation 🙂

@tonsky you have any feelings about this? 🙂

@martinklepsch I think I got it. GCC really doesn't like IIFE's

Try exporting the f (fn ...) let into a separte function with only one binding in the let (thus avoiding IIFE).

Then specify! in there and call that function as the last statement.

(defn hmmmmmm [c]
  (let [f (fn []
            (let [ctr (js* "~{}()" c)]
              (.apply ctr ctr (js-arguments))))]
    (js* "~{}.fooooooo = 1;" f)
    f))
(defn is-this-side-effecting? [spec]
  (let [bf #(-> spec) ;; Avoid IIFE
        c (gf/cacheReturnValue bf)]
    (hmmmmmm c)))
(is-this-side-effecting? {})

So by not using hmmm (great name, right?) and putting it within the let of the lower function you'll get an IIFE and GCC doesn't know it's "just" a function and won't DCE.

The first binding in a let is always good and CLJS won't IIFE it. Hence why it won't work further down.

@rauh ok, managed to reproduce that

(defn- set-meta [c]
  (let [f (fn []
            (let [ctr (js* “~{}()” c)]
              (.apply ctr ctr (js-arguments))))]
    (specify! f IMeta (-meta [_] (meta (c))))
    f))

(defn lazy-build [ctor render mixins display-name]
  (let [bf #(ctor render mixins display-name) ;; Avoid IIFE
        c  (goog.functions/cacheReturnValue bf)]
    (set-meta c)))

@martinklepsch Ie, you got it to DCE? Yay!

yes, DCE working

but idk, feels complex

Well, IMO it's rather nice. GCC correctly detects that all the fn's are not side effecting

You don't need to write the js*, it's just an optimization I have running locally

Whats the difference between (js* "~{}()" c) and (c)?

re complexity: yes maybe the solution is nice but the sole reason for it is a feature that isn’t part of the public API

@martinklepsch (c) will check for the IFn to be there when you enable :static-fns.

I see