styled(Component) does not apply styles to functional component that returns styled-component

[garrettmaring]

Version

Styled Components: 2.1.1
React: 16.0.0

Steps to reproduce

const DataDefinitionListItem = ({ label, value }) => (
  <Flex align="center" direction="column">
    <Flex.Item>
      <DataDefinitionListItemValue>
        {value.toString()}
      </DataDefinitionListItemValue>
    </Flex.Item>
    <Flex.Item>
      <DataDefinitionListItemLabel>
        {label}
      </DataDefinitionListItemLabel>
    </Flex.Item>
  </Flex>
);

export default styled(DataDefinitionListItem)`
  display: none;
`;

The <Flex> component is a styled component. The bug is also reproducible when wrapping the <Flex> inside a <div>.

Expected Behavior

This item should have the style display: none applied to its wrapper element created by styled-components.

Actual Behavior

The style is not applied.

[kitten]

This is expected behaviour since you’re not passing the className prop on: https://www.styled-components.com/docs/basics#styling-any-components

[kitten]

Also btw, as best practice that often helps keep your styling code scalable:

Try to not wrap a structural component (like DataDefinitionListItem) in another styled component.

Styled components are presentational components. They render just some styled elements. While I often makes sense to wrap external, third party components, it doesn’t to wrap your own structural component again.

In short it’s advisable to try to keep to the following structure from top to bottom: container → structural → presentational components

When you think about wrapping your own components, this often moves UI logic around to a couple of places. If you keep them inside their respective structural and presentational components there’s always only one place where stylistic variants and differences (I.e. css classes and styles) can come from

Hope this helps as well :)

[garrettmaring]

Ah, I absolutely just read over that! (Reading examples and not the documentation above is a bad habit). Apologies for missing something in the docs.

That being said, is it really needed to force users to do that on their own? Could it not be a prop that styled components deals with directly?

And thank you for the design tip! :)

[kitten]

@garettmaring it is certainly possible by capturing the output of your component and replacing the React lifecycle for it, but I don't believe that's a good idea 😉 Probably a bad practice entirely

[garrettmaring]

Oh, that's a good point. Appreciate the quick replies!

[sadokmtir]

@kitten May I ask why it is not a good practice to wrap an existing styled component ?
In some cases, you have a component that you use it in several places but sometimes the same component and its logic but just with different style sheet, so doesn't it make sense when you wrap it and pass to it different styles in this case ?

[kitten]

@sadokmtir so to be break my last comment up there (#1226 (comment)) down a little.

There's several ways to alter a StyledComponent once it's created and hence after a while you build up your component library quite a bit. There are some practices that avoid this library becoming less maintainable.

If you wrap some or one StyledComponents in a normal component to add logic, you basically create a structure of StyledComponents around them. This structure is best maintained when all presentational (read classes, styles) concerns are not inside these "structural components". This makes more sense when you think of them in a tree. You have some state in higher components, some structure in the middle, and your styles below. This specialises every part of your application to do one thing in a simple way.

Now, let's say you'd like to extend a structural component. Let's say you have a Menu (normal component) that wraps a couple of StyledComponents. It might be intuitive for you, depending on your thinking / background / etc, to just wrap it: styled(Menu). This is not wrong, and Menu can subsequently accept a className, but the thing to remember is that you're most likely building a component library for an app, not a fully 100% reusable library for npm.

Effectively doing this will mix some presentational (styling) concerns above structural components. This means that in a refactor you now can't rely on a stable order, as outlined in my previous comment.

Instead you could pass props, utilise the theme, react to being wrapped in a certain component. But all in all, I can guarantee you that if you try to adhere to the structure I propose, where it's best to wrap/extend StyledComponents themselves first before trying anything else, you will find your styled-components-driven application to suddenly become a lot more maintainable, even past thousands of styles.

[adamchenwei]

Styled-Component should rename itself as styled-element since it really does not style a real react component, even the simplest functional one ... no?

[kitten]

@adamchenwei very off topic, so please keep these types of things to Spectrum or sth casual like Twitter 😉 but the idea is that it's creating styled components. Even an element will be wrapped as a component.

[marharyta]

I am experiencing the same exact issue with "styled-components": "^3.2.6",
the component style does not apply with the following syntax

import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';

const Button = ({ href, children }) => {
  return <a href={href}>{children}</a>;
};

const StyledButton = styled(Button)`
  color: palevioletred;
  font-weight: bold;
`;

Button.propTypes = {
  href: PropTypes.string,
  children: PropTypes.any
};
export default StyledButton;

[BernabeFelix]

@marharyta pass className to Button

const Button = ({ className, href, children }) => {
  return <a className={className} href={href}>{children}</a>;
};

https://codesandbox.io/s/32n86v90qm

[marharyta]

@BernabeFelix Is that something that is mentioned in the docs? If yes, could you please point me to it? Thank you, it solved my issue.

[kitten]

@marharyta yes, that's under "Styling any components": https://www.styled-components.com/docs/basics#styling-any-components

First paragraph:

The styled method works perfectly on all of your own or any third-party components as well, as long as they pass the className prop to their rendered sub-components, which should pass it too, and so on. Ultimately, the className must be passed down the line to an actual DOM node for the styling to take any effect.

Hope this clarifies it 👍

I'll lock this issue now as it's been long resolved and closed. Please feel free to open new issues for any other questions / issues 😉