Material-ui: Button styled with 'styled' function loses button features when given an internal Link component

[miloofcroton]

When a button is standard (not modified by the styled function), it takes a component={component} prop fine. When it has been modified by the styled function but doesn't have a component, it also renders fine. When it is modified by the styled function and has a component prop, the object reverts to the internal component and loses lots of default button stuff.

Image:

• [x] This is not a v0.x issue.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

The styled function should still work when I pass in a component prop.

Current Behavior 😯

I am currently forced to do styling a different way because it loses button properties when I try to use the styled function and then give it a component to represent internally.

Steps to Reproduce 🕹

Link: https://codesandbox.io/s/pwwj6lwn4m

It's all set up in the code sandbox. The middle element, which is purple but has no button properties, is the example of where it doesn't work.

Context 🔦

I want to tightly couple my styles with my components. I hate having css classes that reference places in the JSX when I can directly give different elements their styles via CSS in JS.

Your Environment 🌎

| Tech | Version |
|--------------|---------|
| Material-UI | v3.9.2 |
| React | v16.6.3 |
| Browser | Chrome 72.0.3626.81 |
| etc. | |

[oliviertassinari]

@miloofcroton This is a property collision problem. We could work around the problem by having the styled() wrapper handle a as property while Button keeps handling the component property. Right now, the styled() wrapper handle the component property directly, the Button is skipped.

[miloofcroton]

Thanks for the response. Is there anything I can do as a work around, or is this something that has to change in the library?

[miloofcroton]

I noticed a similar issue with the Tab element. If you style it and then try to give it a component prop, it goes wonky. The common thread seems to be giving these things component props. Although for both cases for me, they are Link components that I'm passing as that value, so the to value for the Link could also be part of the problem.

[eps1lon]

@miloofcroton This is a property collision problem.

I'm confused about this behavior anyway:

// StyledButton.js
const styles = {}
export const StyledButton = styled(Button)(styles);
// App.js
import { StyledButton } from 'StyledButton.js'
<StyledButton component={Dialog} />

Why would somebody import the StyledButton if the whole Button is then ignored in the render? That's a hole lot of dead code that is bundled.

I looks like the goal is to ultimately do

// StyledButton.js
export const styles = {} // already exported implicitly through the `styled` API
export const StyledButton = styled(Button)(styles);
// App.js
import { styles as buttonStyles } from 'StyledButton.js'
const StyledDialogLikeButton = styled(Dialog)(buttonStyles);
<StyledDialogLikeButton />

[miloofcroton]

@eps1lon

If I understand you correctly, you're asking why I don't apply my styles directly to the 'x' component (where x is the component that I'm currently passing in as the component to the button), correct?

I like button styling for a lot of things, and I want to import the default button styles (not just the custom ones that I set) into the styles of the 'x' component.

I'm still not 100% sure that I understood you, and it's very late here, so let me know if I'm missing something.

[eps1lon]

@miloofcroton It was targeted at @oliviertassinari.

[oliviertassinari]

@eps1lon I don't understand your point. @miloofcroton's use case is legit. You take a button, you override it. You want to maintain its original features. In this case: being able to change the underlying DOM element, from a button to a react-router link.

Why would somebody import the StyledButton if the whole Button is then ignored in the render?

This is the problem, it's not the behavior we want. We want the component property to be handled by the Button, not the StyledButton. This works with styled-components and emotion because they are using a different property name: as, while we use the component property. (A playground with styled-components https://codesandbox.io/s/3r0kvpkp61)

I think that the best path going forward is to rename component -> as in:
https://github.com/mui-org/material-ui/blob/11277a230fdf6971d5ccbbe05f9e71e935b9ee63/packages/material-ui-styles/src/styled.js#L30

---

I wanted to look into the rename of the component property to as for v4. However, given the conflict it would cause with emotion or styled-components, I don't think that we can move forward.

[eps1lon]

My problem is that we have an API that allows to do const Styled = styled(styles)(Component) and later <Styled as='a' /> which renders all the code in Component as dead code. It's an anti-pattern regarding bundle size reduction.

I know that styled-components is doing this. I think we shouldn't repeat the same mistake just so users can use tagged template literals in their files instead of exporting a style object for reuse.

It looks like you're suggesting to rename component in the styled API to as? Could you describe what would happen if I do <StyledButton as="div" component={Link} /> What would this translate to?

[oliviertassinari]

Could you describe what would happen if I do <StyledButton as="div" component={Link} /> What would this translate to?

@eps1lon This is an invalid combination, React should warn an unknown component property applied on a div element.

It's an anti-pattern regarding bundle size reduction.

I doubt that people will use the as property when overriding MUI components. They will use the component property.

I know that styled-components is doing this. I think we shouldn't repeat the same mistake

I don't think that it's a mistake, their motivation is exactly the same than ours for having a component property. Most styled components start from a native element.

[oliviertassinari]

Ok, I definitely think that we should rename the property component -> as in:
https://github.com/mui-org/material-ui/blob/11277a230fdf6971d5ccbbe05f9e71e935b9ee63/packages/material-ui-styles/src/styled.js#L30

@miloofcroton Do you want to work on it?

[miloofcroton]

@olivertassinari

I would be interested in that...

[oliviertassinari]

@miloofcroton It's all good on my side, I would be happy to review your pull request :).

[oliviertassinari]

The problem lies in the fact that the styled component needs to know, beforehand, if its wrapped component implements, or not the component property. The problem would be identical if we were using the same component property name than styled-components or emotion (as). I have done some research, I could find few identical issues:

• https://github.com/styled-components/styled-components/issues/2129
• https://github.com/reakit/reakit/issues/226 (reakit solved the problem by renaming the property)
• https://github.com/styled-components/styled-components/issues/2183

More than ever, I'm confident that we should rename component -> as for consistency with styled-components and for enabling a better component control.

@miloofcroton Do you want to submit a pull request? :)

[oliviertassinari]

As we will defer this problem to jss, we can close