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

Slot component vs useRender hook #1315

Closed
colmtuite opened this issue Jan 9, 2025 · 9 comments · Fixed by #1418
Closed

Slot component vs useRender hook #1315

colmtuite opened this issue Jan 9, 2025 · 9 comments · Fixed by #1418
Assignees
Labels
new feature New feature or request

Comments

@colmtuite
Copy link
Contributor

Feature request

Summary

There is interest from users in having a helper to create their own polymorphic components using render.

Examples in other libraries

https://www.radix-ui.com/primitives/docs/utilities/slot
https://github.com/radix-ui/primitives/tree/main/packages/react/slot/src

Motivation

It's quite a common need when creating a styled component library.

@colmtuite colmtuite added the new feature New feature or request label Jan 9, 2025
@colmtuite colmtuite added this to Base UI Jan 9, 2025
@github-project-automation github-project-automation bot moved this to Backlog in Base UI Jan 9, 2025
@seloner
Copy link
Contributor

seloner commented Jan 9, 2025

Can I work on this? I prob will follow a similar implementation with radix.
What do you think?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 11, 2025

I landed here reading https://adamfratino.com/notes/base-ui-vs-radix-ui from @adamfratino that says

And seeing as how Base UI hasn't included a Slot component

Makes sense. For example, I could imagine all the Material UI components supporting this render prop instead of the component prop, hence having a public API exposed is needed to make it work.

@oliviertassinari
Copy link
Member

Off-topic. I see potential for @samuelsycamore to help with the docs around this feature. I landed on https://base-ui.com/react/handbook/composition and read:

SCR-20250111-bzte

I had to pause and focus to understand. Maybe it's because I have some level of dyslexia or autism. I don't know, something about it made it hard for me.

To compare with https://ariakit.org/guide/composition#changing-the-html-element that documents the same concept:

SCR-20250111-caar

I didn't feel this struggle. "The render prop lets you specify a different HTML element to be rendered instead of the default element" connected instantly for me. I wasn't 100% aware of what the prop was doing until now, so I experienced this closer to what a pristine user would.

@colmtuite
Copy link
Contributor Author

Sure @seloner! I have already added it to the roadmap for our January cycle, so feel free to take it. Thanks!

@colmtuite colmtuite moved this from Backlog to In progress in Base UI Jan 13, 2025
@adamfratino
Copy link
Contributor

I landed here reading https://adamfratino.com/notes/base-ui-vs-radix-ui from @adamfratino that says

And seeing as how Base UI hasn't included a Slot component

Makes sense. For example, I could imagine all the Material UI components supporting this render prop instead of the component prop, hence having a public API exposed is needed to make it work.

@oliviertassinari that's the first and only article i've written 🫣, glad it made some sense! and thanks for tagging me.

+1 the ability to easily apply a render prop to custom components would be a major QOL upgrade IMO.

for example, our team uses a few polymorphic layout primitives (box, stack, group, layer, etc.), currently leveraging Slot. if we migrate to base-ui, keeping the API of those primitives aligned with our library-driven components would be ideal.

@mj12albert
Copy link
Member

+1 the ability to easily apply a render prop to custom components would be a major QOL upgrade IMO.

@adamfratino If you are interested in trying this out, there's now an open PR with a POC and a sandbox for a useRenderer hook that will let you do exactly this: #1418
We discussed this among the team and decided that a hook may be a better abstraction than <Slot/>

@mnajdova mnajdova changed the title Slot utility Slot component vs useRender hook Feb 12, 2025
@mnajdova
Copy link
Member

We have two options on how to provide this feature, hook based and component based. You can check the difference in usage here: https://deploy-preview-1418--base-ui.netlify.app/react/utils/use-renderer#usage

Which API would you prefer?

@58bits
Copy link

58bits commented Feb 12, 2025

We have two options on how to provide this feature, hook based and component based. You can check the difference in usage here: https://deploy-preview-1418--base-ui.netlify.app/react/utils/use-renderer#usage

Which API would you prefer?

I think the hook is nice. There's also nothing to stop us from naming the prop renderAs which might be a nice way to be a bit more explicit about what this prop does .

I wish I'd had the time to look at this more closely but, is it correct to assume that this...

const { renderElement } = useRenderer({
    render: render ?? <p />,
    state,
    className,
    props: otherProps,
  });

... will correctly merge / combined props from both the container component - Text in the linked example, and the passed in render component?

Also I may be wrong, but the component approach would require some clever types and typed refs to play nicely with a polymorphic Slot component?

So overall, I think +1 for the hook. Although we've used the Slot approach previously and it works too.

@mnajdova
Copy link
Member

mnajdova commented Feb 12, 2025

will correctly merge / combined props from both the container component - Text in the linked example, and the passed in render component?

Yes exactly.

Also I may be wrong, but the component approach would require some clever types and typed refs to play nicely with a polymorphic Slot component?

True, if we want to allow just spreading props on the component we will have to either have more complex types, which may cause some issues with TS compilation time (we had that in the past in Material UI), or we could make theм looser, we'll see. For now, let's just focus on DX, to see what makes most sense.

@github-project-automation github-project-automation bot moved this from In progress to Done in Base UI Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants