lilactown / helix

A simple, easy to use library for React development in ClojureScript.
Eclipse Public License 2.0
631 stars 52 forks source link

normalize the class prop #45

Closed lucywang000 closed 4 years ago

lucywang000 commented 4 years ago

To support specifying :class with vectors/lists of symbols. This could make it easier to construct class names conditionally.

{:class (cond-> '[foo bar]
     (some-test?)
     (conj 'barz)}

And normalize the class string:

{:class "foo bar
baz"}

;; becomes
{:class "foo bar baz"}

This could be handy when writing multiline string and still control the 80-chars line limit when there are lots of classes. It's very common when using functional frameworks like tachyons.

tachyons

This happens at macro expansion, so no runtime overhead is added.

tekacs commented 4 years ago

This would be super helpful -- I've currently customized my $ macro to support an arglist-style argument before props that's a class vector.

The rules that I've set up are:

So that these work:

;; ns: namespaced
(def vars-too '[a b c])

($ :div ["some-class" '[things go "here"] (if condition "one two" :three) (when something namespaced/vars-too)])

Keywords are probably unnecessary, but other than that the functionality is helpful.

lucywang000 commented 4 years ago

The behavior looks great now, thank you! Only thing I ask now is a bit of restructuring to colocate all the branches for this inside normalize-class for readability.

@Lokeh that would improve the readability, but that would lose the benefits for compile time optimization, e.g. 1) for a string value, there is no need to emit a string? check at runtime 2) constant lists like '[foo bar] could be turned into a string at compile time.

lilactown commented 4 years ago

@lucywang000 that should still happen; at macro time, it will take the :clj branch of normalize-class which should only emit a runtime normalize-class if it's not a string? or quoted vector.

At least, that was my intention with my suggested edits.

lilactown commented 4 years ago

I think I forgot to add the conditions to the :clj normalize-class. 😅

lilactown commented 4 years ago

I'll accept this PR and play with it a bit. I'll tag you in the PR if I do reorganize the normalize-class logic.

lucywang000 commented 4 years ago

Thanks @Lokeh !