nosco / hx

A simple, easy to use library for React development in ClojureScript.
MIT License
247 stars 16 forks source link

Only munge props when converting to native React elements #40

Closed lilactown closed 5 years ago

lilactown commented 5 years ago

This PR changes the way that hx.hiccup converts props when parsing hiccup, so that it will only do camel->kebab and other munging if the element being passed the props is a keyword (e.g. :div, :span).

It also changes the way hx.react/defnc parses the props object, so that it will not do camel->kebab and other munging.

The following code exemplifies the new behavior:

(defnc Foo [{:keys [text-to-render]}]
  [:div {:style {:color "red"}} "Foo: " text-to-render])

(hx/f [Foo {:text-to-render "This text shows"}])

(hx/f [Foo {:textToRender "This text doesn't show"}])

(defnc Bar [{:keys [textToRender]}]
  [:div {:style {:color "blue"}} "Bar: " textToRender])

(hx/f [Bar {:text-to-render "This text doesn't show"}])

(hx/f [Bar {:textToRender "This text shows"}])

:class is special-cased to be made available as :class-name for backwards compatibility reasons.

orestis commented 5 years ago

I just tried this out with some react-bootstrap components we are using, and it's slightly broken...

It seems like this change doesn't handle converting :class, :style and the other special properties of React to :className, :style with camel-cased names.

So for example:

[bootstrap/Form.Check {:label "Have never logged in"
                              :type "radio"
                              :checked (current-selection :have-never-logged-in)
                              :onChange (on-change :have-never-logged-in)
                              :class form-check-class
                              :id (unique-id 3)}]

yields the warning

Warning: Invalid DOM property `class`. Did you mean `className`?
lilactown commented 5 years ago

That sounds correct - I removed all of the props conversions for Components as elements. For interop purposes it would probably be good to keep around className and style conversions, you're right.

orestis commented 5 years ago

From a quick glance at the React reference (and at the existing source code), it seems like className (class), and style and potentially htmlFor are the only ones that we need to convert.

There's a bunch of others too, but I don't think we need to worry about them.

There's also the case of fully custom attributes, which React expects to be full lowercase (e.g. x-placement or data-something). I don't think we need to do anything there, just pass the original name in converted into a string and be done with it.

lilactown commented 5 years ago

I have added conversion of :class to "className" and :for to "htmlFor", as well as conversion of styles prop to JS, back to props conversion for functions/classes.