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

Styles encapsulation: opt-in inheritance #147

Closed
giuseppeg opened this issue Oct 29, 2016 · 9 comments
Closed

Styles encapsulation: opt-in inheritance #147

giuseppeg opened this issue Oct 29, 2016 · 9 comments

Comments

@giuseppeg
Copy link

giuseppeg commented Oct 29, 2016

This is a feature request or rather an idea – would love to know your opinion.

Like CSS Modules, CSS-in-JS solutions or even BEM and SUIT CSS,
styled components generates unique class names to avoid name clashing and global disasters caused by the CSS cascade property.

People feel pretty confident when using these tools but ignore the fact that there is another enemy around the corner that is CSS inheritance i.e.

styles applied to a component can affect the subtree and even reach the leaves. Properties like color, font-size, font-family are still inherited.

This is good for many reasons but it is an unpredictable side-effect and imho inheritance should be opt-in (instead of opt-out) and predictable. Pete Hunt mentioned it at CSSConf [video] but we were already implementing this idea into SUIT CSS preprocessor and it is now available in v3 [link].

We could reset inherited and non-inherited properties at the root of the component to get styles encapsulation like in ShadowDOM. Descendants can still inherit from the component scope because they only get non-ineherited properties reset – styles won't affect nested components.

The only difference between SUIT and styled components is that in styled components there aren't descendants i.e. each component is a single element without nested stuff. Therefore in order to fit components into a specific context we'd need to have a way to tell that a component is part of a widget or something (in a way a descendant of a scoped component) and reset only non-inherited properties instead.

API Ideas:

import {descendant} from 'styled-components'

// returns a styled component with inherited and non-inherited properties reset
const Button = styled.button`
  border: 1px solid ${color};
`

// returns a styled component with only non-inherited properties reset
const HeaderButton = descendant(Button)

In alternative styles encapsulation could be opt-in

const Button = styled.button`
  /** @encapsulate */
  border: 1px solid ${color};
`
// or

import {encapsulate} from 'styled-components'
const EncapsulatedButton = encapsulate(Button)

Any of the above solutions would just inject (or remove) the following global class names:

.__styled-components-encapsulate-reset-inherited {
  color: initial;
  font-family: initial;
  font-size: medium;
  /* ... */
}

.__styled-components-encapsulate-reset-non-inherited {
  margin: 0;
  padding: 0;
  width: auto;
  /* ... */
}

These two rules should be at the top of the stylesheet so that styled components can override single properties.

@geelen
Copy link
Member

geelen commented Oct 31, 2016

Hmm, this is interesting. But I don't think the Shadow DOM does quite what you think. Shadow DOM makes a boundary that selectors can't cross, but inheritance can. You can use all: initial on an element to override anything inherited, but that's something different (at least I think that's the case, this sort of stuff is hard to know definitively)

I'm actually a big proponent of using inheritance judiciously rather than trying to reset every element all the time. I tend to think people are more worried by the idea of inheritance changing values within their components than the reality of doing so. Personally, I think each "outer component" can simply reset the properties it cares about and pass on the ones it doesn't. That way your "descendants" are totally responsive to the properties already set on the page, which makes them much more reusable.

I gave a talk on this topic a few months back if you're interested: https://www.youtube.com/watch?v=XR6eM_5pAb0

@giuseppeg
Copy link
Author

giuseppeg commented Oct 31, 2016

whoa I had no idea that elements in the Shadow DOM inherit from the parent – I thought that SD was fully encapsulated.

Anyway the point of this feature is to make inheritance predictable by whitelisting inherited properties rather than blacklisting/resetting the rest.

For example button should inherit color, font, line-height but maybe no white-space and others.

I tend to think people are more worried by the idea of inheritance changing values within their components than the reality of doing so.

Agreed maybe opt-out leads to a less painful approach, I haven't made up my mind yet :) Thank you for sharing your knowledge and thoughts! Feel free to close the issue if you want.

@giuseppeg
Copy link
Author

@geelen great talk. Inheritance with custom properties is indeed very powerful and would still work with my solution whose sole purpose is to remove side effects.

With custom properties you would still need to be explicit eg. color: var(--fg, #000) like you would without color: inherit or any other value.

@giuseppeg giuseppeg changed the title Styles encapsulation similar to ShadowDOM Styles encapsulation: opt-in inheritance Nov 1, 2016
@giuseppeg
Copy link
Author

giuseppeg commented Nov 1, 2016

@geelen correct me if I am wrong but

Shadow DOM makes a boundary that selectors can't cross, but inheritance can.

this doesn't apply if we put styles only in the ShadowDOM which was my original idea https://jsfiddle.net/rvnga5mm

<div class="Component">
    #shadowRoot
        <style>div { font-size: 3em }</style>
        <div>Howdy</div>
        <content></content> <!-- content in a way is like React's props.children -->

    <div class="NestedComponent">
        <!-- won't be affected by Component's [non-]inherited props -->
    </div>    
</div>

Resetting everything is a way to get similar level of encapsulation.

@giuseppeg
Copy link
Author

Adding this fyi most of the native elements that use ShadowDOM reset inherited properties - these are UA styles though.

https://jsfiddle.net/o68tg4vo/

@geelen
Copy link
Member

geelen commented Nov 16, 2016

@giuseppeg no it still applies. It's just that the selector in your rule div { font-size: 3em } inside your shadow DOM matches nothing outside, so there's no elements with font-size: 3em with children that can inherit it. If you nest another component within it (or used slots to inject content) then the CSS properties should be inherited as if the shadow boundary wasn't even there.

@giuseppeg
Copy link
Author

giuseppeg commented Nov 17, 2016

@geelen uhm are you saying that it affects the light DOM or nested components in the ShadowDOM? If the latter, agreed otherwise I am confused.

Just to clarify even further, I suggest to reset all of the inherited properties and explicitly set some to inherit.

.foo {
  all: revert;
  color: inherit;
  font-family: inherit;
}

@mxstbr
Copy link
Member

mxstbr commented Jul 1, 2017

Since nobody has asked for this feature in almost half a year it's unlikely we're going to introduce it to the library. We would also break a large amount of existing users with this change, which is not something I'd be particularly comfortable with.

I'll close this issue for now, if you still strongly feel this should be part of styled-components one way or another feel free to comment again or open a new issue!

@mxstbr mxstbr closed this as completed Jul 1, 2017
@giuseppeg
Copy link
Author

yea I also changed my mind in the meantime.. that idea was based on one of the early versions of shadow dom which now instead doesn't block inheritance.

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

No branches or pull requests

3 participants