lilactown / helix

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

Spread props do not work in (auto-defined) factory functions #74

Closed wcerfgba closed 3 years ago

wcerfgba commented 4 years ago

Hello,

I've just noticed that if I define a component like

(helix.core/defnc Foo [{:keys [bar baz]}]
  {:helix/features {:define-factory true}}
  ($ "p" bar baz))

then I cannot pass through props like

(let [props {:baz "test"}]
  (Foo {:bar "yo" :& props}))

instead I must

(let [props {:baz "test"}]
  (Foo (merge {:bar "yo"} props)))

Is this a limitation of factory functions or just a bug? I can live without :& on factory functions if they are not possible, but maybe this should be documented at docs/creating-elements.md#factory-functions ?

Thank you! :smiley:

lilactown commented 4 years ago

This is a limitation of factory functions. Spread props only work with the $ macro, or macros which use the $ macro (e.g. the macros defined in helix.dom).

Since factory functions are, as it says on the tin, functions, they can not do anything at compile time - which is what the $ macro does, rewrite the props at compile time to be akin to (merge {:bar "yo"} props).

The spread props behavior was created to optimize calls to React's createElement function. Factory functions give up this optimization, for ease of typing 😄

Hope that makes sense. Doc improvement pull requests are most welcome!