Lint for tag-based selectors

[ebryn]

I would like to ban styling by tag-based selectors in these pod-based styles.css files. For example, this would not be valid:

h3 {
  color: red;
}

You would be required to chain a class:

h3.my-header {
  color: red;
}

This is my conservative approach, but I'm interested in perhaps completely banning tag-based selectors that don't contain a dash, or maybe having a whitelist of legitimate usages.

Any thoughts?

[jamesarosen]

I might even go so far as to ban h3.my-header, but I understand the "only classes-and-attributes" opinion isn't universal.

[ebryn]

@jamesarosen I agree. My plan is to iterate towards more restrictive CSS, but I know these things may be highly controversial.

[jamesarosen]

Easier to start off more restrictive and loosen!

I think allowing only dasherized tags is a fair start. They're likely to have sufficient semantic meaning.

[runspired]

I disagree with banning tag based styling in these files, or if it is implemented it should allow tag based styling when a custom tag is part of the selector. My particular use case is that I set tagName on many components to be a semantically accurate tag.

e.g. in my current setup I often do something similar to this:

var CarouselComponent = Ember.Component.extend({ tagName : "carousel-component" });

style/components/carousel-component.styl

carousel-component
    font-size 1rem
    makeBlockElement()

carousel-component h1
    font-size 2em
    color #444

carousel-component img
   width 100%
   float left

The advantage of doing it this way is that theming becomes extra-easy. Since only tag selectors are in use, individual carousels can be re-themed without worrying too much about specificity.

[ascot21]

I'm not sure what banning these achieves? Seems like the addon's goal is to allow defining styles in a component. Determining what styles one uses seems very opinionated and not in line with the whole purpose.

[runspired]

+1 to @ascot21 doesn't seem to belong here to me either.

[ebryn]

@ascot21 I may need to have two separate addons but my intent was definitely to bring opinions about CSS to Ember apps. Constraining CSS usage so Ember users don't have to worry about component CSS leaking outside of the component is a huge win.

[gregone]

@runspired using semantically accurate tags in your templates doesn't mean you should use the tag name to style them. CSS best practices have evolved for large apps (SMACSS, OOCSS, BEM), and almost all of them discourage styling based on the tag name.

[jamesarosen]

But wouldn't

h3 { color: green; }

compile to

.my-component h3 { color: green; }

In which case, the only reason to avoid tag names is performance. I tend to avoid tag selectors for that reason, but it's a personal preference.

I'm coming around to allowing anything that's allowed to come after .my-component.

[ebryn]

@jamesarosen yes, it would, but nested components that contain h3s would be styled as well by that rule, which is a no no.

[jamesarosen]

Of course!

[Munter]

Why even bother with cascading style sheets if the goal is to limit styling to only the elements that are part of a component? Why not just inline style all of it to guarantee no leakage? It's probably template generated anyway, so it won't add considerable overhead. Using classnames to try and isolate these styles using individual global namespaces is a huge hack

[ebryn]

I should also mention, I intend to investigate rewriting the user defined class names to autogenerated classes as well. For example:

.foo { color: green; }

could become something like:

.my-component-foo-a34b14 { color: green; }

[raytiley]

@jamesarosen yes, it would, but nested components that contain h3s would be styled as well by that rule, which is a no no.

Could you compile to direct descendent sectors?

.my-component > h3 { color: green; }

I'm also on the fence about limiting to dasherized only. There are some css methodologies like BEM which make use of underscores.

[ebryn]

@Munter my hope is that I can define a constrained enough set of rules that you could flip a switch and we could have the addon automagically inline styles to guarantee no leakage.

[Munter]

If that is the goal, what is the value of even having it in traditional CSS notation in the first place?

[jamesarosen]

@raytiley I didn't mean dahserized classes, just tag names. Sorry if I confused.

[ebryn]

@Munter the same kind of goals that underly our usage of HTML templating languages

[cibernox]

I think I remember that Web components spec had some functionality to "reset" outer styles. If that is true and and is polyfillable, seems ideal.

[ebryn]

@raytiley I want a methodology to rule them all, or make the rules for constraints / autogeneration configurable

[ebryn]

@runspired That pattern of defining a matching tagName will be part of our angle-bracket component syntax by default. When that lands I may want to switch the autogenerated namespaces to utilize the component tag names.

[runspired]

@gregone I wouldn't suggest styling for your site via tag name, but in terms of providing sensible defaults for a component I definitely think you should. The only reason not to is to prevent style leakage, and if we namespace the selectors then we do that already.

namespacing to prevent leakage is a good idea, but it needs to be implemented in a way that doesn't assume the styles for the component that are in the pod are the final version. With the new addon story, addon components are going to be flying everywhere.

@ebryn I also thing Inline styles is a bad idea. Components should be defined within their pod, but their polish should be easily applied elsewhere. Since inline styles always win specificity, this would lead to designers using !important everywhere.

[DrummerHead]

I beliebe that in trying to "baby proofing" it, it ends up in a situation where you have less power and flexibility. If you can write a blogpost (or something longer than 144 chars :P) and make a good case for it, I would be very interested in reading it.

[ebryn]

@runspired namespacing selectors like i am presently is not enough to prevent style leakage. components are nested inside of other components and even @raytiley's suggestion of using > is not enough as subcomponents can still be direct descendents of the parent component.

[DrummerHead]

s/beliebe/believe/g no Justins allowed

[petehunt]

I spoke a bit about how we did it at Instagram last year and I think it's pretty relevant for this discussion: http://youtu.be/VkTCL6Nqm6Y?t=21m9s

tl;dw single class name selectors only (no overrides/tags/etc), prefix all classnames with the name of your module, and try to avoid the cascade where possible. To achieve skinning components should be styled minimally and take class names in as string parameters to be applied to their individual inner parts (but favor composition where possible).

[ebryn]

@petehunt that aligns pretty much spot on with my thoughts as well. thanks for chiming in!

[chadhietala]

Calling @chriseppstein and @eoneill.

[raytiley]

We've been doing what @petehunt said in our app internally and it's been working great for us.

We currently wire it all up manually but @ebryn conventions will make it much nicer.

One thing we've toyed around with is creating a base.scss and a theme.scss. Base has the bare minimum for layout while a theme.scss can pretty things up for app specific usage.

[ebryn]

@raytiley theming is a whole other topic, but i do have thoughts (and ambitions :P) about them as well and want to explore them too

[raytiley]

@ebryn yeah excited to see some conventions around all of this. One idea I had was for addons to have blueprints for theme style sheets. Might be a dumb idea but thought I would throw it out there :)

[chriseppstein]

I have some doubts:

1. Changing the specificity of selectors makes it hard to theme across components. IMO, all classes should be re-written instead of relying on the descendant combinator.
2. Avoiding element selectors with the descendant combinator makes sense especially for application styles, but element (or *) selectors using the child or other combinators seems fine.
3. Especially typographic styles are almost always element based because often content comes from a CMS or some third-party source where you can't control the markup except to know it's a well-formed subset of html. (content tags are usually things like h1..h6, a, p, em, strong, ol, ul, li, hr -- anything you'd expect markdown to produce). for these, often the descendent combinator is the only appropriate one because you can't know how some of these tags might be nested within each other.
4. Any of the arguments you're making about an element name apply to any generic class name (e.g. .last).

IMO, the correct solution is a much harder one than you're going for here. There's not a simple bullet that's going to give you modular css with a few hours of coding. I see a few possible options:

1. Give warnings when selectors from one component might match elements within a nested component based on what you know about how those components are nested in the particular document structures they have. Then give a syntax in sass/less/css that would let the user do the scoping where it's needed. E.g. in Sass you could expose a function component-id() or variable $component-id.
2. Do the selector matching at compile time and inline the styles into the output of the component's template. Then you know for sure that the styles will never bleed because they can't. Note: this also makes theming hard because inline styles are more specific than css selectors. It's unclear how this could work with content that is provided at runtime.
3. Create an optimizer/rewriter that rewrites the css into forms that you can guarantee doesn't bleed across components. It's unclear how this could work with content that is provided at runtime.

For options 2 & 3, you'd probably need a way to flag styles as runtime styles that would effectively leave them alone except for a simple scope that either they provide or the framework does.

[taras]

Browsers provide default style to HTML element with tag css. For example, a div and p are display: block by default and span is display:inline. So, banning this behaviour might be one step too far. It's functionality that needs to be use appropriately.

I like @chriseppstein suggestion because instead of creating hard rules, we can use tooling to educate the user when they're doing something that might have unintended consequences. Knowing about HTML elements that are used in a component allows us to inform the user when a wrong selector is being used and remove CSS that's not used.

Let's say we have a component called blog-post with the following template(using syntax from Element & Fragment RFC):

<element>
  <div class="content">{{content}}</div>
  <div class="comments">
     {{#each comment in comments}}
       <div class="comment">{{comment}}</div>
     {{/each}}
  </div>
</element>

& {
  display: block;
}
.content {
  font-style: italic;
}
.comments {
  margin-left: 20px;
}
.comment {
  border-bottom: 1px dotted grey;
}

In development, we could compile this to

blog-post {
  display: block;
}
blog-post .content {
  font-style: italic;
}
blog-post .comments {
  margin-left: 20px;
}
blog-post .comment {
  border-bottom: 1px dotted grey;
}

For production, we could modify the HTML classes and flatten all of the CSS class names as @petehunt suggests in his video.

  <div class="blog-post_content">{{content}}</div>
  <div class="blog-post_comments">
     {{#each comment in comments}}
       <div class="blog-post_comment">{{comment}}</div>
     {{/each}}
  </div>

.blog-post_content {
  font-style: italic;
}
.blog-post_comments {
  margin-left: 20px;
}
.blog-post_comment {
  border-bottom: 1px dotted grey;
}

[chriseppstein]

@taras I would highly discourage doing something so drastically different in production than in dev like that. Changing the specificity changes the cascade and can have potentially drastic display issues.

[taras]

@chriseppstein fair point.

[taras]

@chriseppstein CSS compilers make changes to CSS but they can't effect HTML because they don't know about it. We have control over both the HTML generation and CSS generation, so we can prevent drastic display issues. Under what circumstances do you think this kind of change would break CSS?

[chriseppstein]

Imagine you have some css like this:

.foo .bar { color: red; }
.baz { color: blue; }

And a document like this:

<div class="foo"><span class="bar baz">I am red</span></div>

Then when you transform .foo .bar into .foo-bar the color of the text "I am red" will change to blue.

[chriseppstein]

I don't believe in perfect software or perfect developers, so all I'm saying is if you're going to do that, do it in dev :)

[eoneill]

First, instead of trying to integrate with Sass/Less/Stylus/etc, should we just focus on leveraging a CSS parsing and doing a simple rewrite on CSS class selectors (note that I think we should only rewrite classes and not try to rewrite other types of selectors).

Sample code time...

[my-component.css]

button { color: black; }
.button { border: 1px solid black; }
.title { font-size: 200%; }

[my-component.hbs]

<element tagName="my-component">
  <h2 class="title">{{attr.title}}</h2>
  {{outlet}}
  <button class="button">Click Me</button>
</element>

[application.hbs]

<my-component title="Hello">
  <button class="button">Click Me Moar!</button>
</my-component>

Now, at build time, we can hash all the class based selectors...

[my-component.css]

button { color: black; } /* note that we leave this alone */
.button-abc1234 { border: 1px solid black; }
.title-def5678 { font-size: 200%; }

And at runtime, HTMLBars can handle hashing the class names. The final HTMLBars output would look something like...

<my-component>
  <h2 class="title title-abc1234">Hello</h2>
  <button class="button button-xyz0987">Click Me Moar!</button>
  <button class="button button-def5678">Click Me</button>
</my-component>

Some things to note:

• HTMLBars will render the original class name, as well as the hashed class name
  • this would allow us to have opt-out (e.g. don't hash my class name) and things still work
• the hashing algorithm used at CSS build time and HTMLBars runtime should be the same
  • roughly hash(className, componentName)
• we get two different hashed button classes
  • button-def5678 is the same as the CSS, this is derived by hashing using the my-component context
  • button-xyz0987 is not part of my-component so it was hashed with a different context (e.g. application, or whatever the component context is)

The general concept here is to make class names non-global by default, but still preserve all the other things we like about CSS.

You could still leverage the cascade by opting out of individual class hashing, but this is now explicit.

[taras]

@chriseppstein your example relies on hierarchy for CSS priority which is a big no no. That kind of use case should be discouraged by linter if it's possible to detect it.

[chriseppstein]

<soapbox> If you're limiting css to the point that it's not like writing css, this tool is not going to go over well, IMO. Embrace CSS for what it is, and make it work well for markup the user writes. If you can't do this, then it's better to develop your own styling language or do something more template oriented and take css selector authoring out of the user's hands. </soapbox>

[taras]

@chriseppstein I actually like CSS, but some features make CSS hard to read, understand and refactor. Mixing HTML hierarchy priority and css order as you showed in your example is one of those "features" that make CSS really difficult to work with. But, I think we're getting off topic of this issue now :)

[gregone]

I’d agree with @chriseppstein that we shouldn’t change the way CSS works. Just enforce best practices and prevent devs from trolling themselves with specificity.

• unique top-level (component) classnames
• shallow nesting
• discouraging (not forbidding) tag selectors
• enabling composition

So, since every component needs to have a unique name, simply relying on 1-level deep nesting should work:

{{!-- app.hbs --}}
<h3>My Green app title</h3>
<my-component>
  <my-sub-component>
</my-component>

{{!-- my-component.hbs --}}
<h3>My Red Component Title</h3>
{{yield}}

{{!-- my-sub-component.hbs --}}
<h3>My Orange subcomponent title</h3>

/* in app.css */
h3{
  color: green;
}

/* my-component.scss */
& {
  border: 1px solid #ddd;
}
h3{
  color: red;
}

/* my-sub-component.scss */
& {
  border: 1px solid #333;
  margin: 10px;
}
h3{
  color: orange;
}

CSS Output:

h3{
  color: green;
}

.my-component{
  border: 1px solid #ddd;
}
.my-component h3{
  color: red;
}

.my-sub-component{
  border: 1px solid #333;
  margin: 10px;
}
.my-sub-component h3{
  color: orange;
}

If we add a linting option to warn or error on tag selectors (and ids), it should be enough to cover the 80% of best practices. If a dev can’t make it work with that, they might want to learn a bit more about css.

[ebryn]

@gregone What you detail is exactly what we have right now. The intent of this issue was to have a discussion around spewing warnings/errors (perhaps even tests would fail?) when a tag-based selector was used inside of a component's CSS. There would be an escape valve, a line-level comment perhaps, to override this behavior, just like we have with linters like JSHint.

The point here was to make you feel bad when you use tag-based selectors, because I believe the 95% case is that you're doing something wrong. The example @gregone detailed above is exactly the situation I want to prevent, as my-component is leaking h3 styling into my-sub-component and I believe almost certainly that people typically don't intend this to happen when building reusable components. I believe the exception is perhaps when building components that are intended to be composed together and I'd rather explore that use case as a separate thing altogether.

[ebryn]

@eoneill You've basically detailed my ultimate plan

[eoneill]

@ebryn awesome, lets make it happen :)

If we can get that approach working, I would argue that strict linting inside of ember-component-css isn't as useful and better achieved by a separate, dedicated addon.

Additionally, we could then consider rewriting all non-class selectors to include classes and HTMLBars could add those classes to all child nodes of the component given the correct context. This still risks selector specificity changes, so this would have to be an opt-in feature.

The caveat here is that this might make HTMLBars too smart.

[ebryn]

@eoneill I've actually started with an easier/opt-in solution by making a class-for helper that you use to get the generated class name.

[eoinkelly]

Instead of banning it, perhaps a better approach would be to encourage the right thing:

1. When my-component.css is generated it could contain some comments that explain best practice e.g.

   /* All selectors in this file will be rewritten to be prefixed with the
     component name e.g. .foo becomes .my-component-foo */
   /* For maximum sanity use only class selectors in this file because
     http://ember-component-css.github.com/docs/why-only-classes.md */
   
   & {
   }

2. If the dev does use a bare tag selector, then emit a warning like; Warning: pods/my-component/my-component.css line 12: Tag selectors are a source of confusion in component CSS. Change to a class selector of if you really need to style *all* button elements thenstyles/tags.cssis a good location - see http://ember-component-css.github.com/docs/why-not-tag-selectors.md for details.

There could potentially have a strict mode that would enforce those warnings but I think we should default to giving amazing warnings :-)

[ebryn]

Closing for now, this may end up being a separate addon.