Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
SevereOverfl0w opened this issue Jun 14, 2021 · 2 comments
Open

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

SevereOverfl0w opened this issue Jun 14, 2021 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@SevereOverfl0w
Copy link
Contributor

helix/src/helix/core.cljs

Lines 131 to 137 in 51e758e

(defn extract-cljs-props
[o]
(when (and ^boolean goog/DEBUG (or (map? o) (nil? o)))
(throw (ex-info "Props received were a map. This probably means you're calling your component as a function." {:props o})))
(if-let [props (gobj/get o "helix/props")]
(assoc-some props :children (gobj/get o "children"))
(bean/bean o)))

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
Copy link
Owner

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.

@lilactown lilactown added bug Something isn't working good first issue Good for newcomers labels Jun 4, 2022
@SevereOverfl0w
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants