Class name composition is harder than it needs to be

[p-himik]

An example of what IMO the usage should end up with:

(defn grandparent [{:keys [class]}]
  [parent {:class [:parent class]}])

(defn parent [{:keys [class]}]
  [:span {:class [:child class]}])

(render [grandparent {:class :grandparent}])

Currently, this results in the class being ultimately set to "child [:parent :grandparent]".
So the users have to use r/class-names every single time they need to combine classes:

(defn grandparent [{:keys [class]}]
  [parent {:class (r/class-names :parent class)}])

(defn parent [{:keys [class]}]
  [:span {:class (r/class-names :child class)}])

(render [grandparent {:class :grandparent}])

Not too harde but harder than it can be.

Another problem with that approach is that the data is composed prematurely. When the "stringification" happens at the very edge of the rendering, the user code can still the full :class data structure as it was created by the app, including all the collections, keywords, strings, nils. Seeing that definitely helps with debugging. It also enables class collection preprocessing in some child component, although I'm hesitant do deem this particular usage as viable.

[Deraen]

I don't see a way to improve this.

:class isn't a special property for Reagent components ([function-name {props ...}] so we can't automatically coerce these values to strings. Currently it is perfectly valid that the :class property value has a clojure datastructure used for something else, this might be quite a rare case but is supported now and changing this would be a breaking change.

[p-himik]

How is it not special if Reagent converts :class to "className"? If it's not special in general, it must be special in some contexts that Reagent is aware of.

To be clear - I'm not saying that Reagent components should start seeing :class as a properly formatted string. I'm saying the exact opposite - Reagent components should see only what's been passed to them, without any conversion, as it is right now. But corresponding React elements should have their className be set to a properly formatted string.

[Deraen]

We only convert :class to :classNames for HTML elements, like :span etc.

From your example it wasn't obvious to me in which step the additional conversion should happen.

We to convert the :class value to string already, like on your example, we support vector values and we convert those (and keywords inside the vector) to strings. Challenge with this is keeping this fast enough, extending this with support for new types might affect even use where user doesn't need to new conversions. class-names function (which reagent is also internally using to merge these class values from html elements) is non-recursive by designs, so it doesn't need to check if each value inside the collection is another collection.

https://github.com/reagent-project/reagent/blob/master/src/reagent/impl/util.cljs#L129-L137

Maybe it would be possible to extend this with additional coll? check in the keep fn and then recursively call class-names for those values, and it would only affect perf when user is already providing collection values to :class.

[p-himik]

We only convert :class to :classNames for HTML elements, like :span etc.

Of course. I think React components are free to treat class they way they please, so nothing can be done in the case of non-HTML elements.

class-names function (which reagent is also internally using to merge these class values from html elements) is non-recursive by designs, so it doesn't need to check if each value inside the collection is another collection.

I started writing an answer to that before noticing that you've said it yourself at the bottom:

Maybe it would be possible to extend this with additional coll? check in the keep fn and then recursively call class-names for those values, and it would only affect perf when user is already providing collection values to :class.

Exactly. So maybe the 1-arg arity can be something like

([class]
   (cond
     (named? class) (name class)
     (coll? class) (string/join " " (map class-names class))
     :else class))