Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

How to handle as prop on styled(Component) components #448

Closed
jjenzz opened this issue Mar 1, 2021 · 10 comments
Closed

How to handle as prop on styled(Component) components #448

jjenzz opened this issue Mar 1, 2021 · 10 comments
Labels
bug Something isn't working P1

Comments

@jjenzz
Copy link
Contributor

jjenzz commented Mar 1, 2021

Is your feature request related to a problem? Please describe.

What I am about to describe is something I have found confusing with styled-components and also exists in stitches and we had someone report this as a bug in our Primitives Discord channel over the weekend.

They tried something equivalent to the following:

const Primitive = ({ as: Comp = 'div', ...props }) => <Comp data-radix-primitive {...props} />;
const Item = styled(Primitive, { color: "red" });
const ButtonBase = styled("button", { fontSize: "16px" });

render(<Item as={ButtonBase}>Loses `Primitive` attrs and functionality</Item>)

A working example of the above code: https://codesandbox.io/s/xenodochial-oskar-ywq1z?file=/src/App.js:43-221

When doing this, the expectation was that Item would become a button with color: red; fontSize: 16px styles but also maintain the data-radix-primitive attribute and Primitive functionality. That is not the case.

The as prop will replace the component passed to styled() entirely with the component passed in the as prop. This tends not to be expected but also seems a bit of an issue for us specifically given that we're advocating this is how people would style Primitives while maintaining the as prop for use in their application.

Describe the solution you'd like

I'm not raising as a bug because honestly, I'm not sure how this should behave.

It behaves the same with styled-components, but you will see that they also provide a forwardedAs prop to achieve what we expect: https://codesandbox.io/s/recursing-zhukovsky-qvkow?file=/src/App.js

My expectation is that it would behave the same as styled-components but I've never been keen on the forwardedAs prop. It might be the only way though.

Describe alternatives you've considered

Hoping the team might have some ideas 🙏

@jjenzz
Copy link
Contributor Author

jjenzz commented Mar 9, 2021

Here's a PoC implementation of how I would expect styled to behave 🙏

https://codesandbox.io/embed/tender-gould-3q05k?file=/src/App.js

@hennessyevan
Copy link

Are there any known workarounds for this? I'm trying to get a styled(Tab, {}) (Tab coming from radix) to work "as" a react router NavLink but I can't seem to combine the props from both Tab and Navlink.

@jjenzz
Copy link
Contributor Author

jjenzz commented Apr 20, 2021

EDIT: I've updated the sandbox and code snippet here with the correct example. My bad 😅

I believe the solution is to use the Primitive as prop instead of the stitches one:

const Tab = React.forwardRef((props, forwardedRef) => (
  <TabsPrimitive.Tab {...props} ref={forwardedRef} as={StyledTab} />
));

const StyledTab = styled("div", { color: "red" });

https://codesandbox.io/s/strange-waterfall-to6fu

On a side note, I'm curious why you are converting a tab to a link? 😛 That sounds like you might be breaking accessibility there. The tab pattern is specifically for controlling display of content on the current page (not for navigation). If you're using the Tab component because you just need something that looks like a tab then it's probably a mis-use of the component https://www.w3.org/TR/wai-aria-practices/#tabpanel

@hennessyevan
Copy link

You're right, (thanks for looking out on this front 😄) I was hoping to just reuse the Tab styles and keyboard navigation but wrap it in a TabNavigation instead. I'm probably better off just sharing the styles and not using the Radix primitive. I was tinkering around with that and I think I expected the props to be merged or something.

@benoitgrelard
Copy link

Just want to leave a not here that this was raised again as an issue in our primitives Discord: https://discord.com/channels/752614004387610674/803656530259738674/844509767187300422

https://codesandbox.io/s/immutable-cloud-jsh7o?file=/src/App.js

@peduarte peduarte added the bug Something isn't working label May 27, 2021
@peduarte
Copy link
Contributor

Possibly related (raised by @smhutch): https://codesandbox.io/s/the-as-prop-f7um6?file=/src/App.tsx

@rafgraph
Copy link
Contributor

@jjenzz nice proof of concept 👍👍 I think we should create a polymorphic api standard that would solve this issue and allow multiple polymorphic components to work together seamlessly. Check out #609.

@benoitgrelard
Copy link

Another person hit that issue when using radix + stitches:
https://discord.com/channels/752614004387610674/803656530259738674/859087164523806730

@trekinbami
Copy link

I think I have the same problem here: https://codesandbox.io/s/cool-andras-9sqjx?file=/src/App.js

@peduarte peduarte added the typescript TypeScript related label Jul 21, 2021
@jonathantneal jonathantneal removed the typescript TypeScript related label Jul 21, 2021
@jonathantneal
Copy link
Contributor

jonathantneal commented Sep 9, 2021

We can move this into a discussion. The interoperability of as with Radix is changing, since Radix is (presumably) dropping the as prop, and there is no built-in mechanism for marking React components are polymorphic.

@stitchesjs stitchesjs locked and limited conversation to collaborators Sep 9, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
bug Something isn't working P1
Projects
None yet
Development

No branches or pull requests

7 participants