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

[styled-engine] Drop withComponent support #27780

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Aug 15, 2021

Breaking changes

  • [styled-engine] Drop withComponent support (#27780) @oliviertassinari
    Use the as prop instead. It's not style destructive when using multiple layers of styled() calls.

Why?

  1. styled-components talks about removing it

Capture d’écran 2021-08-15 à 19 11 05

https://styled-components.com/docs/api#withcomponent

  1. goober doesn't support it

Capture d’écran 2021-08-15 à 19 11 40

https://goober.js.org/

  1. We almost never use it in the codebase, the smaller the API surface, the easier we can make changes.

I have noticed this in #27776.

@oliviertassinari oliviertassinari added the package: styled-engine Specific to @mui/styled-engine label Aug 15, 2021
@oliviertassinari oliviertassinari force-pushed the remove-withComponent branch 2 times, most recently from 6db4277 to 2543a42 Compare August 15, 2021 17:14
@mui-pr-bot
Copy link

mui-pr-bot commented Aug 15, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against e29ffb2

@oliviertassinari oliviertassinari force-pushed the remove-withComponent branch 3 times, most recently from d58bdb6 to e29ffb2 Compare August 15, 2021 18:23

const ItemLink = styled(Item.withComponent(Link), {
const ItemLink = styled(Item, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const ItemLink = styled(Item, {
const ItemLink = styled(({ component = Link, ...props }) => <Item component={component} {...props} />, {

isn't this the equivalent?

Copy link
Member Author

@oliviertassinari oliviertassinari Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine the equivalent is :

Suggested change
const ItemLink = styled(Item, {
const ItemLink = styled(({ component = Link, ...props }) => <Item as={component} {...props} />, {

but there is already an as prop coming from the navigation that conflicts, e.g. as: '/api/accordion' so it doesn't work.

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not even documenting this API anywhere so we should be good to go..

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We almost never use it in the codebase, the smaller the API surface, the easier we can make changes.

👍🏻

tag: Tag,
): StyledComponent<PropsOf<Tag>, StyleProps, Theme>;
}
ComponentSelector {}
Copy link
Member Author

@oliviertassinari oliviertassinari Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to push it the extra mile with the tradeoff of @dimitropoulos in #20012 (comment)?

Suggested change
ComponentSelector {}
ComponentSelector {
/**
* Use the `as` prop instead.
* It's not style destructive when using multiple layers of styled() calls.
*/
withComponent(tag: any): never;
}

Copy link
Member Author

@oliviertassinari oliviertassinari Aug 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it doesn't work in this context. It would silence the missing support in TypeScript.

@oliviertassinari oliviertassinari merged commit f5f5538 into mui:next Aug 16, 2021
@oliviertassinari oliviertassinari deleted the remove-withComponent branch August 16, 2021 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change package: styled-engine Specific to @mui/styled-engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants