sibling selectors with computed props

[ianstormtaylor]

• emotion version: 9.2.4
• react version: 16.0.0

Relevant code:

const Container = styled('div')`
  background: ${props => props.active ? 'red' : 'transparent'};

  & + & {
 	margin-top: 20px;
  }
`

Here I'm trying to style the sibling selector for a given component. Without the computed background it works fine, since the CSS classname will always be the same. But as soon as the computed property is added it seems like that means that the classname will change, depending on props.active.

But if the component with props.active is in the middle of two others, this will also break the & + & since it's no longer referring to all of the component instances.

---

I think the way to solve this would be to have a "computed class" for each component, and also a "base class" that never changes. And the & self selector would always use the base class, so that it's unaffected by computed classes.

This sounds similar to something styled-components added.

---

Or, I might be doing something wrong, let me know! Thanks.

[tkh44]

This should work if you are using the babel-plugin

const Container = styled('div') `
  background: ${props => props.active ? 'red' : 'transparent'};

  & + ${Container} {
 	margin-top: 20px;
  }
`

[emmatown]

It should work if you do this. You need a function returning the component since the component hasn't been declared yet when you reference it.

const Container = styled('div') `
  background: ${props => props.active ? 'red' : 'transparent'};

  & + ${() => Container} {
 	margin-top: 20px;
  }
`

[tkh44]

Interesting, what you said makes sense but I tested it on a local app with the Babel plugin on and it worked 🤷

[ianstormtaylor]

Hey @tkh44 and @mitchellhamilton thanks for the workarounds! As far as I can tell, it would actually then require using the function in both places? Like so...

const Container = styled('div') `
  background: ${props => props.active ? 'red' : 'transparent'};

  ${() => Container} + ${() => Container} {
 	margin-top: 20px;
  }
`

This starts to feel really hacky though.

---

More generally... this feels like something where the default behavior isn't right. It's really hard for me to think of a case meriting the current behavior, where any change to any of the dynamic props will change the & + & in mind-bending ways.

It seems like & should be pegged to refer to the component itself—regardless of the current values of dynamic properties.

Do you agree? Is this something that can be fixed?

[aaronjensen]

How would I use this with the object syntax? Is that possible?

[tkh44]

I think using a function value and object styles it would look like this. I haven't tested it though.

const Container = styled('div')((props) => ({
 background: props.active ? 'red' : 'transparent',
 [`${Container} + ${Container}`]: {
   marginTop: 20
 }
}))

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[ianstormtaylor]

This still feels like an API flaw to me. All of the workarounds are unintuitive (and hard to determine what even the correct one is), whereas the current behavior is almost never going to be what people want in the first place.

[Andarist]

I think you are right, not sure how to reliably generate unique stable class names for scoping (targeting &) purposes without babel plugin (emotion@10 has a goal of babel plugin being totally optional, only providing build-time optimizations and such).

[emmatown]

I disagree, I think the behavior here is correct. & has to target the hashed class name otherwise the styles would be the same for all of the component instances. The way I think about it is that every style block is implicitly wrapped in &. As an example, how would it work in this case if & referred to the component selector class name?

let Comp = styled.div`
  &:hover {
    color: ${props => props.hoverColor};
  }
`;

[aaronjensen]

What if a new syntax was introduced like && ? It could mean any variation of this

[Andarist]

@mitchellhamilton u are right (as always 😅 ), maybe we could somehow detect problematic patterns (with eslint plugin?) and warn about them? The problematic pattern here would be i.e. & + & not followed by a block with dynamic interpolation

[ianstormtaylor]

@mitchellhamilton that's a great point. What do you think of && (or similar) like @aaronjensen proposed? It seems like there should be a "stable" way to refer to the component's class without the confusing dynamic functions. So maybe the current distinction isn't wrong, but there's still a hole in the API.

[emmatown]

& is pretty much just replaced with .css-hash so && turns into .css-hash.css-hash so that wouldn't work but tbh I don't really understand why using () => SomeComponent is confusing?

[wongm3]

@mitchellhamilton When I use () => SomeComponent in typescript I'm getting an error

Argument of type 'TemplateStringsArray' is not assignable to parameter of type 'Interpolation<undefined>'.
  Type 'TemplateStringsArray' is not assignable to type 'ObjectInterpolation<undefined>'.
    Types of property 'filter' are incompatible.
      Type '{ <S extends string>(callbackfn: (value: string, index: number, array: readonly string[]) => value is S, thisArg?: any): S[]; (callbackfn: (value: string, index: number, array: readonly string[]) => unknown, thisArg?: any): string[]; }' is not assignable to type 'string | string[]'.
        Type '{ <S extends string>(callbackfn: (value: string, index: number, array: readonly string[]) => value is S, thisArg?: any): S[]; (callbackfn: (value: string, index: number, array: readonly string[]) => unknown, thisArg?: any): string[]; }' is missing the following properties from type 'string[]': pop, push, concat, join, and 15 more.

Is there something I'm missing?

[Andarist]

The issue got stale and I think it's appropriate to close this now. Thank you for your understanding.

[ianstormtaylor]

What ever happened to the && suggestion? Not sure why @mitchellhamilton says it wouldn't work, seeing as you could just replace && first and replace & after.

Still seems like a footgun in the API to me.

[Andarist]

There are existing suggestions to use && to overcome specificity issues. If anything this would have to use different token than & as it has well~ established semantics and we wouldn't like to mess with this now.