Emotion: Incorrect handling of restyled components with CSS selectors with nested components

[mdevils]

Current behavior:

If you style already existing styled component, it will lose all styles that base component received via CSS selectors with nested components.

To reproduce:

import styled from '@emotion/styled';

const Link = styled.a`
    color: red;
`;

const LinkContainer = styled.div`
    ${Link} {
        color: blue;
    }
`;

const StyledLink = styled(Link)`
    border: 1px grey solid;
`;

ReactDOM.render(
    <LinkContainer>
        <StyledLink>I must be blue</StyledLink>
    </LinkContainer>,
    document.getElementById('root')
);

Expected behaviour:

In this example StyledLink should be blue, but appears to be red.

styled-components handles this case properly: https://codesandbox.io/s/styled-components-playground-forked-v3829?file=/src/index.js

Environment information:

• react version: 16.13.1
• @emotion/react version: 11.1.4

[Andarist]

I'm on a fence regarding this - I could very much see this working as intended. Philosophy of SC and Emotion differs slightly and in general, we try to discourage some patterns that, in our eyes, lead to less maintainable styles.

On the other hand - this seems like a reasonable expectation given that we already support nesting, targeting other components etc. To make it work we'd have to compose targets for isReal components here: https://github.com/emotion-js/emotion/blob/400c6f72905a4676f852b09405421a7ca8d7a3a2/packages/styled/src/base.js#L37

Thoughts about this @mitchellhamilton ?

[Slapbox]

I always assumed the reason this wasn't supported was related to performance.

In my view, the ideal would be that a restyled component would inherit the nesting rules of the component that's acting as its base, but maybe there are some maintainability issues I'm overlooking?

If it can be supported without hurting performance I'm definitely for this.

[mitchellhamilton]

This sounds good. My general thoughts on styled(SomeStyledComponent) are that it should be equivalent to styled(forwardRef((props, ref) => <SomeStyledComponent {...props} ref={ref} />)) and here we're doing the wrong thing so we should fix it.

There aren't really performance concerns with this, it's just adding some extra class names.

[iChenLei]

It looks like be fixed in latest version ? https://emotion.sh/docs/styled#targeting-another-emotion-component Maybe maintainer can close this issue? @Andarist

[penx]

I think this is the same issue that is illustrated by this sandbox?

https://codesandbox.io/s/k08pmjrl5r

From what I remember, this was raised on emotion slack a couple years ago, though I can't access it anymore!

[Andarist]

@penx this is a different issue - you are referring to & being based on the hashed styles rather than based on the component identity. This is by design and won't change.

@iChenLei the issue can be seen here: https://codesandbox.io/s/recursing-bassi-s6809?file=/src/index.js so it's still there.