Deprecate .extend in favour of only styled(Component)

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?

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)

@mxstbr

@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?

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.

@mxstbr

@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

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

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?

Yeah exactly.

@mxstbr

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 👍

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

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

We had a similar convo last year: #1087

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

@probablyup

@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?

Sounds perfect

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.

Is it still the plan to have styled(Component) detect if Component is a styled-component, and if so, apply the .extend logic under the hood?

We're getting the deprecation warnings, but we can't actually upgrade until the .extend functionality is restored under the hood of styled(), as was suggested in the original description of this issue.

Current compatibility issues with .withComponent are highlighted below:

const Button = styled.button`
  // Imagine some styles here
`;

const ExtendedButton = Button.extend`
  color: green;
`;

const StyledButton = styled(Button)`
  color: green;
`;

// FAILS :(
const StyledButtonLink = styled(StyledButton.withComponent('a'))`
  font-weight: bold;
`
// PASSES :) (because we used .extend)
const ExtendedButtonLink = styled(ExtendedButton.withComponent('a'))`
  font-weight: bold;
`

Here's a code pen demonstrating where this breaks down, and why we can't upgrade our app:

https://codepen.io/greypants/pen/KxaBvx?editors=0010

@greypants

Hey @greypants, could you see the convo here? #1956

If you're using withComponent for the purpose of switching out what physical element is being rendered, a polymorphic component base makes much more sense.

@probablyup

@probablyup woaaaaaaah... that pattern is awesome, thanks!

So the answer then, is "No"? The .extend functionality is being removed completely, not moved back into styled().

Does that mean .withComponent should also be deprecated in favor of the polymorphic component pattern, since it's incompatible with styled()?

Does that mean .withComponent should also be deprecated in favor of the polymorphic component pattern, since it's incompatible with styled()?

We haven't made a formal decision on it yet, but probably. I'm doing some experimentation to see how difficult it would be to bring the polymorph functionality in as a first class prop API in styled

Hello,
It seems that without the .extend functionality, there is no attribute validation within styled-components.

So the following breaks:

  it('should not adopt an invalid attribute', () => {
    const renderedComponent = mount(<CompA attribute="foo" />);
    expect(renderedComponent.prop('attribute')).toBeUndefined();
  });

 // compB is style('section')
 it('should render an <section> tag', () => {
    const renderedComponent = shallow(<CompA />);
    expect(renderedComponent.type()).toEqual('section');
  });

// Expected value to equal:
//      "section"
//    Received:
//       [Function StyledComponent]

With the component:

const CompA = styled(CompB)`
  height: 100%;
  color: black;
`;

Fixed styled components warning

@RIP21

@RIP21 thanks so much for that codemod! I've moved it to the styled-components organization and packaged it into a single executable for nicer usage: https://github.com/styled-components/styled-components-codemods

Will publish a first beta version to npm to try it out 🎉

@mxstbr

@mxstbr glad to hear! If any issues let me know! :)

Fixed styled components warning

Apologies for posting here but I have tried to search through issues and the change log but would appreciate some clarification.

At what version did styled() get updated to deprecate .extend?

Obviously in v4 .extend is no more and styled() "folds" the functionality when trying to style a styled component.

I can see in v3.4.3 that a warning was added when using extend -- does that mean at this point styled() is handling styling styled-components the way extend used to?

We are using extend in a number of places, in apps with a range of versions (back to v2) and am unsure about when I can swap out our .extends in our shared components.

Thanks!

AFAIK styled behaves the same way as it did before. Follow the instructions in codemods repo here in this organization to find out the tiny difference. Long story short it's dangerous when used in conjunction with "withComponent" otherwise quite the same behavior. 24 paź 2018 06:54 "Laurie Jones" <notifications@github.com> napisał(a): Apologies for posting here but I have tried to search through issues and the change log but would appreciate some clarification. At what version did styled() get updated to deprecate .extend? Obviously in v4 .extend is no more and styled() "folds" the functionality when trying to style a styled component. I can see in v3.4.3 <https://github.com/styled-components/styled-components/blob/master/CHANGELOG.md#v343---2018-08-21> that a warning was added when using extend -- does that mean at this point styled() is handling styling styled-components the way extend used to? We are using extend in a number of places, in apps with a range of versions (back to v2) and am unsure about when I can swap out our .extends in our shared components. Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1546 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADwe71UEnPV8lVrUEAnYqCJyDGmq1fgjks5un_JugaJpZM4SPAgL> .

@RIP21

Hey @RIP21 Thanks for the quick reply 🙂

I am confused. Surely styled() can't work the exact same way it always has otherwise the requirement for .extend would still exist/never have existed?

In my experience using styled(SomeStyledComponent) results in multiple sc- classnames and styles being applied (e.g. sc-jtRfpW sfcle sc-elJkPf iJAdYA) with one bringing the original styles and then the other the "overrides". The issue that extend has always fixed for me was that the css for these classes can be in the wrong order. So your overrides appear before the default styles in the stylesheet and therefore do not override.

.extend _merges these styles into the one classname and avoids this whole source-order issue.

@lauriejones

@lauriejones no, the refactor that "folds"/merges styles into each other was only released in v4, although I'm pretty sure your code should still work in v3 if changed to styled(Comp) from .extend.

@lauriejones

I believe I'm running into the same issue that @lauriejones described. After upgrading and moving over to styled in v4, I'm running into specificity issues all over the place. It's hard to tell if it's because something extend was doing was masking actual issues, but it's certainly frustrating to debug.

Is it still the plan to have styled(Component) detect if Component is a styled-component, and if so, apply the .extend logic under the hood?

We're getting the deprecation warnings, but we can't actually upgrade until the .extend functionality is restored under the hood of styled(), as was suggested in the original description of this issue.

Current compatibility issues with .withComponent are highlighted below:
const Button = styled.button`
  // Imagine some styles here
`;

const ExtendedButton = Button.extend`
  color: green;
`;

const StyledButton = styled(Button)`
  color: green;
`;

// FAILS :(
const StyledButtonLink = styled(StyledButton.withComponent('a'))`
  font-weight: bold;
`
// PASSES :) (because we used .extend)
const ExtendedButtonLink = styled(ExtendedButton.withComponent('a'))`
  font-weight: bold;
`
Here's a code pen demonstrating where this breaks down, and why we can't upgrade our app:

https://codepen.io/greypants/pen/KxaBvx?editors=0010

use as replace withComponent

Navigation Menu

Repository navigation

Deprecate .extend in favour of only styled(Component) #1546

50 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Deprecate .extend in favour of only styled(Component) #1546

Description

Activity

mxstbr commented on Feb 22, 2018

kitten commented on Feb 22, 2018

mxstbr commented on Feb 22, 2018

kitten commented on Feb 22, 2018

mxstbr commented on Feb 22, 2018

pke commented on Feb 22, 2018

mxstbr commented on Feb 22, 2018

kitten commented on Feb 22, 2018

mxstbr commented on Feb 22, 2018

quantizor commented on Feb 28, 2018

kitten commented on Feb 28, 2018

mxstbr commented on Feb 28, 2018

pke commented on Feb 28, 2018

50 remaining items

greypants commented on Aug 31, 2018

quantizor commented on Aug 31, 2018

greypants commented on Aug 31, 2018

quantizor commented on Aug 31, 2018

theboyWhoCriedWoolf commented on Sep 10, 2018

mxstbr commented on Oct 1, 2018

RIP21 commented on Oct 1, 2018

lauriejones commented on Oct 24, 2018

RIP21 commented on Oct 24, 2018

lauriejones commented on Oct 24, 2018

mxstbr commented on Oct 25, 2018

mmcc commented on Feb 22, 2019

mimi4Real commented on Nov 13, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions