Don’t join nil classes into classes string

[tonsky]

It started with @fotoetienne noticing in tonsky/rum#99 that

[:.c1 { :class nil }]

and

[:.c1 { :class (when false "...") }]

yield different results. This is unfortunate, because in Rum’s server-side rendering, I do not use precompilation step and render HTML based on its value only.

In Sablono, on the other hand, end result might depend on the compilation mode selected:

(macroexpand '(html [:.c1 { :class nil }]))
;; => (js/React.createElement "div" #js {:className "c1"})
;; => <div class="c1"></div>

(macroexpand '(html [:.c1 { :class (when false "...") }]))
;; => (js/React.createElement "div" #js {:className (sablono.util/join-classes ["c1" (when false "...")])})
;; => <div class="c1 "></div>

(note the extra space after c1).

This patch fixes the issue by patching sablono.util/join-classes to ignore nils and bringing it on par with the strategy used in all-literal expansion of html macro.

I thing you’ll agree that precompilation strategies should be there for speed optimization only, and should yield totally equal results no matter which mode is used.

[r0man]

@tonsky Thanks!

[r0man]

Released as 0.7.5.

[tonsky]

Thanks!

On Mon, Oct 3, 2016, 19:25 r0man [email protected] wrote:

Released as 0.7.5.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#132 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AARabGDd3l35v3QJPlqZ37PwA8HpC-2Cks5qwPQxgaJpZM4KMFeS
.