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

Proposal: Scoped CSS #666

Open
patrick-steele-idem opened this Issue Apr 10, 2017 · 23 comments

Comments

Projects
None yet
@patrick-steele-idem
Copy link
Contributor

patrick-steele-idem commented Apr 10, 2017

style {
    div.__colors {
        /* ... */
    }

    ul.__colors {
        /* ... */
    }

    __colorLI {
        /* ... */
    }

    __disabled-button { 
        color: grey;
    }

    __enabled-button { 
        color: yellow;
    }
}

<div.__colors>
    <ul.__colors>
        <li.__colorLI>
            Red
        </li>
    </ul>

    <!-- Scoped styles can also be passed to other UI components: -->
    <fancy-button disabled-class=style('disabled-button') enabled-class=style('enabled-button')>
        Click me
    </fancy-button>
</div>

NOTE: It would also be possible to use unscoped CSS alongside scoped CSS

The Marko CSS preprocessor would produce the following CSS code:

div.colors_abc123 {
        /* ... */
}

ul.colors_abc123 {
        /* ... */
}

.colorLI_abc123 {
        /* ... */
}

.disabled-button_abc123 { 
    color: grey;
}

.enabled-button_abc123 { 
    color: yellow;
}

NOTE: The CSS code that Marko preprocessors then would go through any user CSS processor specified (style.less, style.scss, etc.)

Finally, the Marko compiler would provide a style function variable similar to the following:

var style = marko_style('_abc123');
// style('colorLI') --> 'colorLI_abc123'
@austinkelleher

This comment has been minimized.

Copy link
Member

austinkelleher commented Apr 11, 2017

I could see this getting messy very quickly. I would much rather like to have a scoped attribute like Vue. With the above implementation, you could mix the scoped and non-scoped CSS, which is much messier. Additionally, when a component is created, often times there will be component-specific styling. This means that there will be a lot of underscores in component styles.

Mixed syntax can be messy

In the following example, it's difficult to see which specific styles are scoped and which specific styles are global without scanning the entire block:

// A mix of scoped and global styles
style {
    .non-scoped {
        /* ... */
    }
    div.__colors {
        /* ... */
    }

    ul.__colors {
        /* ... */
    }

    .more-non-scoped {
        /* ... */
    }
    __colorLI {
        /* ... */
    }

    __disabled-button { 
        color: grey;
    }

    .even-more-non-scoped {
        /* ... */
    }

    __enabled-button { 
        color: yellow;
    }
}

Scoped Attribute

In the following example, you can easily see which styles are scoped and which styles are global:

style.scoped {
    .scoped-class { 
        /* ... */
    }

   div.colors { 
        /* ... */
   }
}

// Global styles can be included in the component too
style {
    .non-scoped-class { 
        /* ... */
    }

   div.another-non-scoped { 
        /* ... */
   }
}

You could use less or an another CSS extension as an additional attribute as usual:

style.less.scoped {

}

style.less {

}
@charlieduong94

This comment has been minimized.

Copy link
Collaborator

charlieduong94 commented Apr 11, 2017

Having a dedicated section for scoped attributes would be a lot nicer than prefixing each class with __. I think most people would prefer to use only scoped css for their components and having to write __ for every class in both the css and the template can get quite annoying after a while.

Although when mixing global and scoped styles with @austinkelleher 's approach, there could end up being some confusion about what classes are scoped or not when looking at a pretty involved template with lots of styling going on. But then again, Vue users doesn't seem to be too bothered by that.

@sebastianmacias

This comment has been minimized.

Copy link

sebastianmacias commented Apr 13, 2017

I agree with the scoped attributes like:

style.less.scoped {

}

style.less {

}

style.scss.scoped {

}

style.scss {

}

Instead of:

    .global-button-selector { 
        color: grey;
    }
    __disabled-button { 
        color: grey;
    }

which in my opinion looks a bit messy and doesn't feel concise like most of the framework.

@patrick-steele-idem

This comment has been minimized.

Copy link
Contributor Author

patrick-steele-idem commented Apr 13, 2017

Sorry guys, but I am going to have to have a dissenting opinion here. We considered the Vue.js approach before creating this proposal, but there are some serious drawbacks with the Vue.js approach:

Vue.js approach drawback: scoped styles must be separated out from unscoped styles

It forces scoped styles to be separated out. For a single file UI components that is fine because it is just a separate block in the single file. However, that would be a problem when the developer wants to split the styles out to a separate style file. Do we allow style.less + style.scoped.less? That would be suboptimal because now styles are split across multiple files. A separator within the file could be used but that introduces more things to learn and the separator could be problematic (do we use a CSS comment?). You could argue that unscoped styles would not be needed alongside scoped styles, but I think that would be an odd restriction.

Vue.js approach drawback: Marko must understand each compile-to-CSS language

Another drawback with the Vue.js approach is that it requires that Marko to be able to deeply understand and parse the style source in order to rewrite the styles. This means that Marko would need to have custom support for each render-to-CSS language (Less, Sass, Stylus, etc.) language so that it could render the style code into CSS so that it could then be deeply parsed to figure out all of the CSS class names and then modified to add an attribute selector. I don't think we want to go there.

For example, given the following:

style scoped {
    .list-container:hover {
        background: orange;
    }

    @media (min-width: 250px) {
        .list-container:hover {
            background: orange;
        }
    }
}

<div>
    Hello World
</div>

Marko would need to figure how to preprocess the possibly non-CSS style source to produce the following CSS output:

.list-container[data-m-21e5b78]:hover {
    background: orange;
}

@media (min-width: 250px) {
    .list-container[data-m-21e5b78]:hover {
        background: orange;
    }
}

We actually don't want Marko to render compile-to-CSS languages to CSS because we want that to be handled by the asset pipeline/bundler (e.g. Lasso or Webpack). This is because we allow global imports for Less and Stylus (and possibly others) and Marko would not have that context.

Contrast that with the proposal I gave above:

style {
    __list-container:hover {
        background: orange;
    }

    @media (min-width: 250px) {
        __list-container:hover {
            background: orange;
        }
    }
}

<div.__list-container>
    Hello World
</div> 

With a little regular expression magic, the __ prefixed variables could be easily rewritten to the following:

.list-container_21e5b78:hover {
    background: orange;
}

@media (min-width: 250px) {
    .list-container_21e5b78:hover {
        background: orange;
    }
}

The rendered HTML would be the following:

<div.list-container_21e5b78>
    Hello World
</div>

In addition, since we would use a special prefix, Marko would not need to deeply parse the CSS to find all of the scoped CSS class names to be added to the output HTML.

Vue.js approach drawback: scoped style classes cannot be passed to nested components

In my proposal above scoped style classes can be passed to nested UI components:

style {
    __disabled-button { 
        color: grey;
    }

    __enabled-button { 
        color: yellow;
    }
}

<div>
    <fancy-button disabled-class=style('disabled-button') enabled-class=style('enabled-button')>
        Click me
    </fancy-button>
</div>

With the Vue.js approach your best option would probably be to use less efficient and less reliable descendent selectors:

style scoped {
    .my-component .disabled-button { 
        color: grey;
    }

    .my-component .enabled-button { 
        color: yellow;
    }
}

<div.my-component>
    <fancy-button disabled-class='disabled-button' enabled-class='enabled-button'>
        Click me
    </fancy-button>
</div>

I'm making the assumption that the output CSS code would be the following:

.my-component[data-m-21e5b78] .disabled-button { 
    color: grey;
}

.my-component[data-m-21e5b78] .enabled-button { 
    color: yellow;
}

Vue.js approach drawback: not compatible with multi-file adaptive styles

At eBay, we support splitting out styles into multiple files for building adaptive UIs so we might have the following:

  • style.mobile.less
  • style.mobile.ios.less
  • style.less
  • ...

In order for that approach to work with the Vue.js approach, Marko would somehow need to figure out how to look at all of the possible style files to figure out if a particular CSS class name is a scoped CSS class name or not so that it can add the extra data-* attribute to the HTML elements that use those scoped class names.

So you don't like underscores?

We also discussed the following option based on title case:

style {
    .ListContainer:hover {
        background: orange;
    }

    @media (min-width: 250px) {
        .ListContainer:hover {
            background: orange;
        }
    }
}

<div.ListContainer>
    Hello World
</div>  

I don't know how I feel about that, but I could definitely be onboard with it. I think the regular expression might be a little less reliable but we could put in a few restrictions to avoid that problem. We recommended the __ prefix because it would fit into any compile-to-CSS language and __ is commonly used to marko properties in JavaScript as private. A pro for the title case is that .ListContainer looks okay, but .__list-container looks bad. A con with the title case is that I don't like having to type uppercase characters (shift key), but "_" requires shift as well so that is super minor.

It's an implementation detail, but the hash could be based on the file system directory path. That is, for any file in the directory we could figure out what the hash would be regardless if we are looking at /path/to/style.less or /path/to/style.mobile.less. It could then be the job of the asset pipeline/bundler to add the hashes to the CSS code (instead of Marko doing that work).

Summary

  • What I proposed above is a combination of scoped CSS and CSS modules
  • It fits in nicely with any compile-to-CSS language without Marko having to deeply parse the compile-to-CSS source code
  • It works with multi-file and single-file approaches
  • It's easy to explain (a unique hash is added to scoped class names that have a special prefix or that are somehow recognizable)

Still think the Vue.js approach is better? Any other thoughts or proposals?

@gilbert

This comment has been minimized.

Copy link
Contributor

gilbert commented Apr 13, 2017

How about double dot?

..list-container:hover {
    background: orange;
}

..list-container..tasty {
    background: chocolate;
}

@media (min-width: 250px) {
    ..list-container:hover {
        background: orange;
    }
}
@patrick-steele-idem

This comment has been minimized.

Copy link
Contributor Author

patrick-steele-idem commented May 4, 2017

@gilbert It's subjective but I am not sure double dot looks better than .__my-class or .MyClass and I suspect that the double dot will impact tooling and syntax highlighting for the render-to-CSS languages such as Less or Sass, but I have not verified (..my-class is not valid CSS but both .__my-class and .MyClass are valid CSS and should not cause any linting and syntax highlighting issues).

@charlieduong94

This comment has been minimized.

Copy link
Collaborator

charlieduong94 commented May 5, 2017

I talked to @austinkelleher about this a while back and I've warm up to the idea of keeping scoped and non-scoped css classes under the same style block. I think that making class names that start with a capital letter scoped would end up working well. It feels less yucky compared to .__.

There are languages out there that have different functionality for things that start with a uppercase or lowercase letter. For example, functions, structs, and struct attributes in golang that start with a capital letter are exported and are available outside of the package they are defined in, while lowercased variants are kept within the scope of the package. Although this is backwards from what was proposed (and go is very different from css), I don't think the concept would be too foreign.

If marko ended up going this route, I think that it would have to be an opt in feature. It could end up breaking layouts for some people.

@gilbert

This comment has been minimized.

Copy link
Contributor

gilbert commented May 5, 2017

You know, double underscore looks weird at first, but looking back on this thread a few weeks later it doesn't look bad at all. I think I prefer it since you can tell something different is happening, as opposed to the capital letters that look like a naming convention.

@zephraph

This comment has been minimized.

Copy link
Contributor

zephraph commented May 27, 2017

Note: I'll probably update after I get more sleep and have time to think

I'm kind of wracking my brain on this one. On one hand, easy of implementation is obviously critical. On the other, this doesn't feel like a solid solution.

Main Concern: Tight coupling between scoped nature of css and markup

If I started off with

<button.primary/>
.primary {
  color: blue;
}

but I decided suddenly that there was a conflict and I needed to scope it.

.__primary {
  color: blue;
}

If I forget to update the markup or miss a reference then there could be a non-obvious style bug introduced.


I know that's very likely obvious, but the above combined with the fact that the __ needs to be added for everything scoped just seems error prone to me.

Modified Vue-like approach

Vue takes a pipelined approach.

scoped styles -> vue-loader -> preprocessor -> internal vue-loader postcss plugin

Using a grouped approach like @austinkelleher mentioned would require something similar.

Implementation

In a single file marko component

<button.primary/>

styles.scoped.less {
  button.primary {
    color: blue;
  }
}

Generated output

button.primary[data-m-3ds24df] {
  color: blue;
}

The flow here would be more or less the same as vue's.

In a split component

style.mobile.css (Phase 0 -- source)

scoped {
  button.primary {
    color: blue;
  }
}
.global-class {
  color: red;
}

Phase 1 -- marko transform

@scoped(data-m-3ds24df) {
  button.primary {
    color: blue;
  }
}
.global-class {
  color: red;
}

Phase 2 -- postprocessor
Phase 3 -- marko-scoped-postcss

button.primary[data-m-3ds24df] {
    color: blue;
  }
.global-class {
  color: red;
}

Intended Results*

  • Solves "scoped styles must be separated out from unscoped styles."
    • Separation exists still, but can be localized to a file
  • Solves: "Marko must understand each compile-to-CSS language."
    • Look for a common denotation in each of the files like @scoped or something similar but unique.
  • Doesn't solve: "scoped style classes cannot be passed to nested components."
    • I don't think this is a best practice to begin with, but it's still obviously an issue.
  • Might solve: "not compatible with multi-file adaptive styles."
    • I would be very interested to know how this works to begin with. Regardless, after marko figured out what styles that component depended on it'd have to check for @scoped. If that was present, you'd just add the attribute directly onto every direct node of that component. (I'm not sure but I believe that doesn't extend to children).

*Note: I'm making a lot of assumptions here that I don't have all the info to back up.

Granted, I understand that this is significantly more complex than the proposed solution. Technically, given @patrick-steele-idem's note about the implementation details of the proposed solution I could see it just being a standalone webpack/lasso plugin anyway.

@sebastianmacias

This comment has been minimized.

Copy link

sebastianmacias commented May 27, 2017

@zephraph thanks for bringing attention to this topic again. I really don't see the issue with having marko support multiple CSS preprocessors.

Having to install the desired preprocessor: SCSS, Less, PostCSS or Stylus via NPM so it's available to compile styles.scoped.less, styles.scoped.scss, etc is a minor trade-off in order to improve readability and improve the framework's usability.

@ramses0

This comment has been minimized.

Copy link

ramses0 commented May 28, 2017

I am strongly of the opinion that all styles in a component should be scoped to that component by default.

Instead of .__foo to indicate that the .foo class is private, I would prefer to have to prefix something like .GLOBAL.foo to escape the particular component.

Consider this option:

style {
    .foo { color: red; }
    i { color: blue; }
}

global-style-exports {
   i { color: pink; }
}

<div class="foo">
  <i>Hello</i> <b>World</b>
</div>

Much discussion about CSS on the internet revolves around "global by default" was a misfeature in CSS (similar to it being in a misfeature in JS). In the above example- it should be hard for styles to escape from the component.

@zephraph

This comment has been minimized.

Copy link
Contributor

zephraph commented May 29, 2017

@ramses0, I really like the idea of all component styles being scoped by default. That would definitely be a breaking change though.

@guilhermeaiolfi

This comment has been minimized.

Copy link

guilhermeaiolfi commented Aug 24, 2017

I don't really know why anyone would put global styles inside a component file. All styles inside the component definition should be scoped. For me, it would be a GOOD thing to NOT allow globals there. Therefor no extra syntax needed. You can put those anywhere else if you like, it would/should be considered a best practice to do so. Imagine how hard it would be to discover why your page is messed up when you include a bunch of components in your page.

Here is the Svelte wiki as ref: https://github.com/sveltejs/svelte.technology/blob/master/guide/03-scoped-styles.md

@gilbert

This comment has been minimized.

Copy link
Contributor

gilbert commented Aug 28, 2017

Other prior art: styled-jsx. Theirs is scoped by default, and they provide a :global modifier:

<style jsx>{`
  /* "div" will be prefixed, but ".react-select" won't */
  div :global(.react-select) {
    color: red
  }
`}</style>

or, if you want all to be global:

<style jsx global>{`
  .this-is-global {}
  .this-is-global-too {}
`}</style>
@zephraph

This comment has been minimized.

Copy link
Contributor

zephraph commented Aug 28, 2017

@guilhermeaiolfi I'm inclined to agree. I think the invert of the vue approach is a more solid option, like @gilbert suggested above.

Scoped by default, global only if explicit. That means you have to know the option exists and specifically apply it. I'm not necessarily for a complete restriction of global styles altogether though. I understand it's a bad practice in most cases, but there are likely a small number of valid use cases.

Again it goes back to it being a breaking change. It'd either need to be behind a flag until the next major release or released as a standalone plugin that integrates with marko and can be used optionally.

@ramses0

This comment has been minimized.

Copy link

ramses0 commented Sep 1, 2017

@austinkelleher ... where's the scoped style discussion then? Seems like both #666 and #825 are closed?

@zephraph

This comment has been minimized.

Copy link
Contributor

zephraph commented Sep 1, 2017

@ramses0 This is it. It's still open.

@mlrawlings mlrawlings added the proposal label Sep 30, 2017

@patrick-steele-idem patrick-steele-idem changed the title Proposal: Scoped CSS using "__" prefix Proposal: Scoped CSS Oct 31, 2017

@patrick-steele-idem

This comment has been minimized.

Copy link
Contributor Author

patrick-steele-idem commented Oct 31, 2017

Just a quick update: we have had various discussions and we are leaning towards following the semantics of CSS encapsulation with the shadow DOM. That is, we want to support the following:

style.scoped {
  b {
    color: red;
  }
}

<b>This will be red and bold tags outside this component will not be impacted!</b>

Additional thoughts:

  • We want to move to "scoped" being the default. style.global { } could be used for global styles (the current behavior
  • We could use marko.json to opt-in to scoped CSS in Marko 4 and show a deprecation warning if the new behavior is not opted into
@andresilvasantos

This comment has been minimized.

Copy link

andresilvasantos commented Oct 31, 2017

Using a css preprocessor, less for example, would result in style.less.scoped?

@patrick-steele-idem

This comment has been minimized.

Copy link
Contributor Author

patrick-steele-idem commented Oct 31, 2017

Using a css preprocessor, less for example, would result in style.less.scoped?

We want to support scoped styles even if a preprocessor is used. We still need to work out the details because this needs to play nice with Webpack, Lasso, etc. and we want to avoid Marko having to be aware of all CSS preprocessor languages. To support scoped CSS the option we are leaning towards is to put a unique component-specific identifier in an attribute on all of the HTML elements to limit the scope of CSS selectors. PostCSS supports parsing most popular CSS syntaxes (for purposes of rewriting CSS selectors), but I don't think Less is supported. We may or may not end up using PostCSS and, instead, defer the work to a Webpack loader or a Lasso plugin. Details still need to be worked out...

@JonShort

This comment has been minimized.

Copy link

JonShort commented Nov 17, 2017

Came across this discussion after noticing some style clashes in my marko project (I had assumed styles were being scoped by default).

Like other commenters I feel Vue-loader's scoped & module attributes work really well as extensible & unopinionated solutions to this issue.

Styles being scoped by default would make marko a bit more opinionated (maybe too much so?), with the style.global attribute being used more like Styled Components' injectGlobal helper method.

@ramses0

This comment has been minimized.

Copy link

ramses0 commented Nov 18, 2017

@JonShort - Much discussion about CSS on the internet revolves around "global by default" was a misfeature in CSS (similar to it being in a misfeature in JS). In the above example- it should be hard for styles to escape from the component.

If there were such a thing as a "component marketplace" then those components MUST use "scoped by default" in order to prevent style clashes. No two thingies can own h1.fontFamily in a sane way.

IMHO, marko component styling should unabashedly fall into two distinct camps that cannot be intermixed:

  1. Component styles must be "scoped" by default and can be as specific as possible

  2. Global styles MUST NOT have any class-name selectors (only parent/child/sibling/nth, etc).

This means:

/* explicitly global, not button.wide, not button.default, not button.primary, etc */
GLOBAL.button { width: 100%; height: 50px; }

 /* implicitly local */
h1 { font-size: 5px; }

(at a minimum, components in a component marketplace MUST be divided into: "local styles only" and "also uses global styles", where there is social pressure to use scoped / local / unique styles if you want your component to get relatively high usage).

@mauricionr

This comment has been minimized.

Copy link

mauricionr commented Jul 27, 2018

any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.