lilactown / helix

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

Factory component doesn't work with HOC that supplies props #84

Open SevereOverfl0w opened 3 years ago

SevereOverfl0w commented 3 years ago

https://github.com/lilactown/helix/blob/51e758eb4c5e64794d2042168d00b12ea4143adf/src/helix/core.cljs#L131-L137

If I define a factory component, that also uses a HOC which provides props (in this case withTooltip https://airbnb.io/visx/docs/tooltip which the docs say are deprecated but is still used in the examples) then the extra props provided by the HOC aren't included.

Maybe the props need to be bean'd as well?

(merge (bean/bean o) (assoc-some props :children (gobj/get o "children")))
lilactown commented 2 years ago

when you use cljs-factory, it puts the props map passed to the factory function under a single prop "helix/props".

If an HOC adds more props, then it won't be included in that map.

You can see in the snippet you posted that if the props object passed to extract-cljs-props has the "helix/props" key, that it will prefer that over anything else contained in the props object.

I think the right thing to do here is to merge more than just "children" with the helix props map, but I'll have to test that out a bit to make sure it doesn't have any unforeseen consequences.

SevereOverfl0w commented 2 years ago

Incidentally, I ran into this again recently using ant design. It's form library clones the children and injects onChange and value props.