Deprecate .extend in favour of only styled(Component)

[pke]

The FAQ explains the difference between styled() and .extend. However I wonder if it would be possible to take the decision out of the devs hand and have styled check for the handed in Component if its already a styled component. If it is .extend it otherwise create a new class.

Was this considered at one point?

[mxstbr]

This was how it originally worked, actually—and honestly, I'm not sure it was the right decision to split them. This is the most common confusion for developers, so I think we should move back to the original version and deprecate .extend in the next major. (with a codemod, obviously)

[kitten]

@mxstbr Totally agreed! We've discussed removing it before—and there was some general concensus—so I think it's time to pull the trigger.

Shall we deprecate it in the next minor release?

[mxstbr]

Let's deprecate and remove from the docs in the next major and make sure we have a codemod in place, then remove the code entirely in the next major after that.

[kitten]

@mxstbr Do you have any reason to believe that we need to keep it around for another major version? 😅 I think that won't actually make a huge difference

[mxstbr]

I don't think folks will be particularly happy about us suddenly throwing warnings in a minor version upgrade.

[pke]

Thanks for considering this simplification of the API surface. I agree such removal of API should not happen in a minor update. Please stay true to semver. The behaviour of styled can be changed in a minor version but the deprecation warning could be introduced in the next major RC.

On the other hand why not release a new major version right away. What to wait for?

[mxstbr]

Yeah exactly.

[kitten]

Yea, I don't have a problem with issuing a major version asap. However, semver doesn't restrict deprecations to major releases. That'd be quite extreme as you'd then during a typical release flow have to release two major bumps—one potentially without breaking changes—just to get new APIs in and remove old ones.

Quote:

When you deprecate part of your public API, you should do two things: (1) update your documentation to let users know about the change, (2) issue a new minor release with the deprecation in place. Before you completely remove the functionality in a new major release there should be at least one minor release that contains the deprecation so that users can smoothly transition to the new API.

https://semver.org/

But anyhow, I'm fine with both, but I'd like to coordinate a new release with #1493 and all related deprecations (injectGlobal). So I'd love to push out a major only when #1493 is done, and deprecate in a minor, but I'll leave the call up to you, @mxstbr :+1:

Btw I'll rename the issue to represent the new plan.

[mxstbr]

Yep, sounds very reasonable to batch that together with #1493! Maybe also #1171? I'd love to land that, personally.

[quantizor]

We had a similar convo last year: https://github.com/styled-components/styled-components/issues/1087

I'm definitely in favor of getting rid of .extend from the public API.

[kitten]

@probablyup @mxstbr to reiterate a little, I'd wait until both aforementioned PRs are merged and available at which point we can safely deprecate .extend, injectGlobal, and keyframes (if an alternative will be available) in a minor. Then in the next major we can remove them.

But we could already start a pending PR that adds the deprecation warnings I suppose?

[mxstbr]

Sounds perfect

[pke]

Reading #1087 I see back then there were strong arguments by members of the project against flattening the API. Good to see decisions can be reverted if they seem to confuse to many users (!) of the library even if the owners initially thought broadening the API surface was a good thing.

[kitten]

@pke to add some more depth to this prior discussion, what @probablyup was seeing became an increasingly large problem on the implementation side—kudos for seeing this so early btw—and while we preferred the API, over time it became clear that we didn’t even agree ourselves on its practical usage and use cases. Together with having problems with it (component IDs), and having no technical reason for it (specificity correctness is guaranteed, so why concat css?) this is proving to be the way forward.

Hope this dimes up everyone’s opinion on the matter :)

[tysonmatanich]

If I am following this correctly, moving forward styled(Fruit) would work how Fruit.extend does today?

const Fruit = styled('div')`
  color: green;
`;
const Orange = styled(Fruit)`
  background: orange;
`;

const Fruit = styled('div')`
  color: green;
`;
const Orange = Fruit.extend`
  background: orange;
`;

I'm all for the simplification, however it would be great if the original styles didn't have to be duplicated which is the same issue I have with how .withComponent() works. Obviously in my sample it’s not a big deal. However, if Fruit had tons of styles and needed to be extended many times, then it seems inefficient to duplicate the styles each time instead of just having these styles defined on a single CSS class.

[denkristoffer]

How would you style a styled component created with withComponent? Currently I sometimes do

const Fruit = styled.div``

const Orange = Fruit.withComponent('span').extend``

Will the following work instead?

const Orange = styled(Fruit.withComponent('span'))

[mxstbr]

@denkristoffer yes.

[pungggi]

hi, just to add my 2 cents.. I totally agree, this is indeed confusing when starting with styled-components. so i like the idea, as a consumer, not to think about what kind of components I am extending, but here it comes...

The documentation stated for a while i think? that .extend is the reccomended way.. but now the 'recommended' way will be deprecated? Why not just leave..both.. so that it it doesnt matter wichone you choose. So, eliminating the confusion, YES, but leave the choice (personal flavour i would say) to use styled(Component) or Component.extend, even if it does the same. Personally i like the second version, because its natural to start thinking wich Component to use, and then what you want to do with it (thank you intellisense..).. and you do not need the import styled from 'styled-components'..

[ckeeney]

If extend is deprecated and removed, will there by an analogous .attrs() for styled?

[denkristoffer]

@ckeeney attrs() works with styled() as it is:

const A = styled(B).attrs({
  style: ({ opacity }) => ({
    opacity,
  }),
})`
  color: red;
`

[ckeeney]

That's great news. Thanks, I must have missed that in the docs and thought it only worked on extend

On Mon, Apr 16, 2018, 10:05 AM Kristoffer notifications@github.com wrote:

@ckeeney https://github.com/ckeeney attrs() works with styled() as it is:

const A = styled(B).attrs({ style: ({ opacity }) => ({ opacity, }), })color: red;

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/styled-components/styled-components/issues/1546#issuecomment-381676625, or mute the thread https://github.com/notifications/unsubscribe-auth/AIZDIhTDsDYeGMZiwZxqAsWqQ-NNIRc-ks5tpM9cgaJpZM4SPAgL .

[ms88privat]

We should be careful here. extend has not the same behaviour as styled. In our case we grep the component displayName in our visual tests. There it makes a difference what we use. If we use extend, it will not get a new displayName, but with styled it does. E.g.:

Will result in:

[ckeeney]

@ms88privat apart from slightly different stack traces and component names, is there any practical difference here to regular users of styled-components?

[tysonmatanich]

@ckeeney Yes, a larger response size for server rendered sites since duplicate styles are defined instead of sharing the same CSS class.

[kizu]

If I understand correct, making styled(Foo) to be equal to Foo.extend if Foo is already a styled-component would seriously break one case. Which is a contextual usage of a component.

Example: having one template component for buttons: Button. Then we can create new versions of buttons by using NewButton = styled(Button) and get one important feature: whenever we would use ${Button} in selectors, they would target any buttons: both simple Button components and modified NewButton ones. This can be crucial for a lot of use cases.

I'm really against completely removing this feature. While I'm ok for making default behavior of styled(Foo) the same as for Foo.extend, and in that case, there would be a need to add an escape hatch for those who would need to have the current behavior.

[evan-scott-zocdoc]

@kizu a new stable classname is created for each individual styled component class, have no fear

[kizu]

The fear is that & > ${Button} inside styles of some other Foo component won't target a NewButton component.

[evan-scott-zocdoc]

For that case, I'd recommend tagging the base component with some well known class or attribute and referring to that. We don't formally support inheritance in component selectors.

e.g.

const BUTTON_CLS = 'button';

const Button = styled.button.attrs({ className: BUTTON_CLS })`
  color: red;
`;

const OtherButton = styled(Button)`
  color: blue;
`;

const Wrapper = styled.div`
  .${BUTTON_CLS} {
    background: green;
  }
`;

<Wrapper>
  <Button>Hi</Button>
  <OtherButton>Hello</OtherButton>
</Wrapper>

Both would have green backgrounds.

[kizu]

Sorry, but there is literally a section dedicated to this method in the documentation: https://www.styled-components.com/docs/advanced#referring-to-other-components, so it is strange to hear that this is something which is not “formally supported”.

As this is already in the doc, and people already use this method which documentation calls as allowing for “extremely powerful composition patterns”, this could be a really big hit to a lot of projects.

[evan-scott-zocdoc]

There is nothing in that document about inheritance. We support component selectors on specific styled-components. If you make a new styled component off another one, the second is not targeted by a component selector on the first.

[kizu]

Ok, I understand what you're talking about, but still — right now this works this way, there are projects that rely on this feature and, yes, 4.0.0 could be breaking in this regards, but without a built-in escape hatch developers would need to cheat and make it so styled-components won't understand that you're passing an existing styled component to styled(). No one would change the existing behavior that just works and does the work perfectly to some classes that are not guaranteed to be unique.

Mixing multiple classes on one element is a really powerful feature and it is strange that it is not supported. Ok, it obviously has its drawbacks, and that is why I'm ok with the default behavior to be the same as extend.

[evan-scott-zocdoc]

Mixing multiple classes on one element is a really powerful feature and it is strange that it is not supported

What do you mean? styled-components let's you do whatever you want. It just generates a stable unique className per styled component class and appends it to the rendered DOM. That's what is used when composing a component selector.

[evan-scott-zocdoc]

The difference between Thing.extend and styled(Thing) is that Thing.extend makes a copy of all the styles of Thing and assigns a single new dynamic className for them.

If you're referring in code to the dynamic classNames generated by a styled-component and not the stable one (Thing.styledComponentId), I highly recommend avoiding that as it isn't part of the API of this library and could change at any time.

[tysonmatanich]

However this duplicates the styles so they are defined on separate class names instead of either sharing a single class name or having a selector that contains multiple class names (.a, .b { }). Has anyone studied the performance impact this has for both client and server rendered sites?

[kizu]

Yes, I'm acutely aware of how extend and styled() work. I'm not referring in code to the dynamic classNames, and I have already written that I use it per the docs I have mentioned — by using ${MyComponent} as a part of the selector.

The problem, again, is that the change in 4.0.0 makes styled-components to treat all other styled-components in a special way from any other components. It would dismiss the original components' stable className.

There are cases when we need both pairs of classNames: those from the original component, and the new ones. We need both stable classNames on one element.

I fell like I'm repeating myself.

[quantizor]

This API has been removed in the v4.0 branch per the roadmap. We'll add a deprecation message to v3.x in an upcoming minor release.

[sdalonzo]

Could the docs get updated too? I've been following https://www.styled-components.com/docs/faqs#which-one-should-you-use and have created unnecessary technical debt for my team.

[mxstbr]

@sdalonzo PR to the docs would be much welcome: https://github.com/styled-components/styled-components-website

[elektronik2k5]

I love the work that's done here, and deprecation warnings are important, but:

1. The new warning is really spamming Jest test output. Can we somehow show it only once? I think that's how it works in the browser, but not in Jest.
2. Why use console.error for a warning? Is it for the stack trace (console.warn shows it too)? Is it for increased visibility? I believe those aren't good reasons. It should be a warning. Fixed in styled-components@3.4.5, thanks to @probablyup !

Thanks again! :+1:

[mxstbr]

@elektronik2k5 PRs fixing those issues welcome!

[quantizor]

@elektronik2k5 I think it's because Jest resets the module registry regularly and when it requires the file fresh, the "once" shortcircuit gets reset since the whole function is recreated. Switching it to a warning sounds fine though.

[quantizor]

Warnings have been made to use warn https://github.com/styled-components/styled-components/commit/fcf6f3804c57a14dd7984dfab7bc06ee2edca044

[Sly777]

Should I update the documentation with this deprecation in "extending styles" page? Because there is nothing about our old friend ".extend" (or I couldn't find it)

Also I wrote small regex to move it to new structure.

Search: (\w*.withComponent\('\w*'\)|\w*.|\w*.\w*.)(?:\.extend`)

Replace: styled($1)`

It works on vs code and atom.

this regex supports;

ComponentName.extend` ==> styled(ComponentName)`
ComponentName.withComponent('span').extend` ==> styled(ComponentName.withComponent('span'))`
ComponentName.ChildName.extend` ==> styled(ComponentName.ChildName)`

It doesn't support X.extend`...`.withComponent('div'); usages.

[kachkaev]

@Sly777 there is at lease one use case to watch out for if I remember things correctly:

const X2 = X.extend`...`.withComponent('div');

should become

const X2 = styled(X.withComponent('div'))`...`;

https://github.com/styled-components/styled-components/issues/1880#issuecomment-414078886

[Sly777]

Aaa I see. I didn't know that usage way.

[IanEdington]

I'm seeing this warning pop up a lot. Is there a codemod for this?

[RIP21]

@mxstbr is there is a need to write a codemod? I can write one with pleasure (babel-codemod) :)

[quantizor]

@RIP21 please feel free! We'd then want to add a link to it in the FAQ section of the website

[mxstbr]

A codemod would be awesome!!!!

We'd then want to add a link to it in the FAQ section of the website

...and the release notes for v4 :+1:

[sbrichardson]

If not mentioned, the codemod would also need to add the import to any files?

import styled from 'styled-components'