Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Remove Component As Selector #329

Closed
tkh44 opened this issue Sep 23, 2017 · 17 comments · Fixed by #334
Closed

RFC: Remove Component As Selector #329

tkh44 opened this issue Sep 23, 2017 · 17 comments · Fixed by #334

Comments

@tkh44
Copy link
Member

tkh44 commented Sep 23, 2017

Right now this works.

const Link = styled('a')`color: blue;`

const CoolBox = styled('div')`
  ${Link} {
    color: red;
  }
}

@mitchellhamilton and I would like to remove this for version 8.

  1. Adds too much complexity for its value.
  2. The babel plugin would become optional if we removed this feature.

@mitchellhamilton can expand a bit more on this, but I never use this feature and think of it as a crutch.

Please let us know what you think.

@apostolos
Copy link
Contributor

Personally have never used it. I'm not in favor of creating dependencies between components like that. I use props or semantic class names like:

const Link = styled.a`
  color: blue;
  &.danger {
    color: red;
  }
`

Will we still be able to nest selectors created by css?

const link = css`
  color: hotpink;
`;

const paragraph = css`
  color: gray;

  .${link} {
    border-bottom: 1px solid currentColor;
  }
`;

@tkh44
Copy link
Member Author

tkh44 commented Sep 23, 2017

Yes, selectors based on css calls will still work because once they are called the class name is static. styled calls rely on the presence of props in order to calculate the final styles. Because of this we have to attach a unique id at compile time. This unique ID is what is used as the selector.

@hamlim
Copy link
Contributor

hamlim commented Sep 23, 2017

I think it would be fine to remove, however in general composition of components normally means some kind of style overriding happening for nested components.

It would be nice if there was an escape hatch (or other patterns) that are well documented to get to the same result.

@tkh44
Copy link
Member Author

tkh44 commented Sep 23, 2017

As for an escape hatch, it would be as simple as this.

const Link = styled('a')`color: blue;`

const CoolBox = styled('div')`
  .link {
    color: red;
  }
}

<CoolBox>
  <Link className="link">
</CoolBox>

Keep in mind that if link was a class name generated by emotion composition works even better.

const Link = styled('a')`color: blue;`

const redLink = css`
  & .link {
    color: red;
  }
}

const CoolBox = styled('div')`
   ${redLink};
}

<CoolBox>
  <Link className="link">
</CoolBox>

@tkh44 tkh44 mentioned this issue Sep 25, 2017
3 tasks
tkh44 pushed a commit that referenced this issue Sep 25, 2017
* Remove component selectors

* Remove tests for component as selector closes #329

* Remove reference to component selectors

* Update snapshots
@emmatown emmatown mentioned this issue Sep 28, 2017
3 tasks
@mxstbr
Copy link
Contributor

mxstbr commented Sep 28, 2017

The babel plugin would become optional if we removed this feature.

I'm not across emotion internals, why can't you make this work without the Babel plugin?

@emmatown
Copy link
Member

@mxstbr We could make it work without the babel plugin with component selectors but SSR hydration wouldn't work and there will always be edge cases around where people use styled and removing component selectors means components will always render deterministically whether the babel plugin has touched them or not.

We also couldn't generate unique component selectors at build time for withComponent from a babel macro since we don't know where withComponent will be called.

@markphilpot
Copy link

We use this style of composition extensively. Being forced to specify className seems contrary to the entire purpose of encapsulating CSS in JS. Is there any plan to have a dedicated plugin to handle this case or is this a use case that emotion doesn't plan to actively support?

@tkh44
Copy link
Member Author

tkh44 commented Oct 9, 2017

@markphilpot I don't recommend using the className hack. I meant it as an alternative.

We've tried a couple of different methods to get this correct and the only way we've found to get it right is using the babel plugin to attach a unique className to each styled component. This was too much overhead to maintain. Both @mitchellhamilton and I find this to be an anti-pattern so this is extremely low on our priority list. I would accept a PR that got this working without the babel plugin and covers all the selectivity edge cases. If this is something you are passionate about I can't wait to see the PR.

@markphilpot
Copy link

I'm curious why you think it's an anti-pattern? Is there a better alternative? I'm open to other ways of organizing my UI but I feel I would need something like this capability to correctly manage hover states.

@cheapsteak
Copy link

We're planning a migration from SC to emotion, and will unfortunately have to fall back to the className hack due to lack of component as selector support

I wish someone would could add it back as an optional feature (similar to the styled.div syntax)

@emmatown
Copy link
Member

@cheapsteak It is now supported when babel-plugin-emotion is used

@ersinakinci
Copy link

My comment is coming a bit late, but I'm also curious why this is considered an anti-pattern. For me, composition using components as selectors seems like the main benefit of CSS-in-JS (echoing @markphilpot). @mitchellhamilton @tkh44 would love to hear your reasoning.

@Andarist
Copy link
Member

Andarist commented May 4, 2020

We tend to believe that creating this kind of dependencies between components tends to lead to less maintainable styles, multiple sources of truths, and breaks encapsulation. Popular approaches for CSS styling such as BEM also avoid this - partially for the mentioned reasons.

@ersinakinci
Copy link

ersinakinci commented May 4, 2020

@Andarist, do you have any specific examples or blog posts, etc. that you could share of how this kind of pattern becomes problematic? I couldn't find anything. The way I see it, using components as selectors promotes maintainability:

  • The component is the single source of truth for everything relating to that component, both its business logic state and its visual state. Sort of analogous in my mind to the ducks pattern in Redux: grouping things together that have traditionally been split apart in the name of encapsulation, but actually when you look at it in a certain way, there's a strong argument to be made that they belong together (i.e., is there usually any difference between a component's state and its visual state, and if not, why should they be kept separately?)
  • Tracing styling dependencies becomes much simpler (only one set of files to track, no need to keep class names in sync with component names).
  • Feels very React-like in terms of composition.

I hear you about BEM, but I see one of the benefits of CSS-in-JS as not having to use approaches like BEM...:-) I love BEM whenever I start a project and I love the idea of separating visual elements/atoms/whatever from the components that they get applied to, but eventually the mental discipline required to keep the two hierarchies in my mind takes a toll. One of the things that makes CSS-in-JS special is that it allows you to safely (for the most part) merge those hierarchies into one without worrying about conflicting global styles and specificity (for the most part). I guess that makes me lazy or bad at CSS, but life is short and I have only so much that I can worry about.

At the risk of sounding naive, if I were to use BEM, then what are the benefits of using Emotion or Styled Components? Not being sarcastic, just genuinely curious about how people merge the two approaches.

I've been using Theme UI, which I think does a nice job on cutting down on duplication. Theme UI and other similar frameworks offer opinionated primitives that use a theme object, which I find replaces shared CSS TTL's and is more maintainable. I've added a handful of primitives to their list, and whenever I need to introduce some kind of minor variation, I simply do styled(Primitive)`...` . However, I'm not that experienced with CSS-in-JS and my project's codebase is modest. I'm curious if you find this to be a sustainable approach.

Btw, thanks to you and to everyone else who contributes to this excellent project ❤️.

@Andarist
Copy link
Member

Andarist commented May 4, 2020

Feels very React-like in terms of composition.

IMHO it's less React-like as you effectively bypass React, its state and props to affect the rendered visual output. It happens outside of React - sort of magically. I'm talking about this from the targeted component's perspective. When you do this it has no knowledge of being targeted. When this pattern is overused you might also end up in specificity wars, which we effectively try to eliminate altogether. It might happen that you target some component (for example conditionally, so its not apparent at first that is being targeted when editing that targeted component) adding a particular property to it and later you want to add the same property to the component itself.

I guess that makes me lazy or bad at CSS, but life is short and I have only so much that I can worry about.

I feel you, I'm terrible at CSS 😅 - so I actually value all constraints put on me when authoring styles because they lead to styles about which it's way easier to reason about.

At the risk of sounding naive, if I were to use BEM, then what are the benefits of using Emotion or Styled Components? Not being sarcastic, just genuinely curious about how people merge the two approaches.

The main advantage for me is that with Emotion you don't have to use CSS preprocessor to automate mundane tasks and you can create abstractions around styles in JavaScript (full-fledged language).

I've been using Theme UI, which I think does a nice job on cutting down on duplication.

Yeah, that's a great project 👍

@ersinakinci
Copy link

You raise some good points, @Andarist. Now that I think about it, I've come across some instances where a target could be specified in multiple ways when using a component as a selector, which introduced some ambiguity and potential specificity conflicts. I think that if I had used the css prop and shared styles, I probably wouldn't have had that problem.

I've been reading more about the library and it seems like composition with CSS TTL's is actually quite convenient. I might do that, although I'm not sure what file structure would be best (separate styles directory? Styling files alongside my components? Separate exports from within my component files?)

@Andarist
Copy link
Member

Andarist commented May 5, 2020

I might do that, although I'm not sure what file structure would be best (separate styles directory? Styling files alongside my components? Separate exports from within my component files?)

Rather hard to advise on that - it all depends on your preferences and the codebase. Do whatever makes sense to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants