rtfeldman / elm-css

Typed CSS in Elm.
https://package.elm-lang.org/packages/rtfeldman/elm-css/latest
BSD 3-Clause "New" or "Revised" License
1.23k stars 196 forks source link

Should we support something like `@extend` or `composes` in addition to mixins? #38

Closed rtfeldman closed 8 years ago

rtfeldman commented 8 years ago

Both SASS's @extend and CSS Modules's composes have to do with using CSS classes to reuse code. Classes are more limited than mixins (for example, they can't incorporate pseudo-classes or pseudo-elements), but whereas mixins disappear after compilation, classes stick around; the browser is actually aware of them. Is that a good justification for expanding their use as a method of style composition?

@extend is a notorious footgun. There's an entire article on why never to use it. (tl;dr from the article itself: "Extending doesn’t necessarily help file weight, contrary to the saying. Extending doesn’t work across media queries. Extending is not flexible. Mixins have absolutely no drawback.") A cursory Google search reveals several other articles raising red flags about @extend.

I personally don't use @extend for these reasons, and don't see any reason elm-css should support that footgun in the first place.

composes declares that a certain class is always to be used in conjunction with another. For example (with some hand-waving) if you're using CSS Modules and you say "btn-warning composes btn" then when in your code you use "class='btn-warning'" you'll actually get "class='btn btn-warning'" in the DOM.

If you already have mixins (which we do), then the benefits of composes seem dubious. If you're using gzip, there's a good chance the way mixins duplicate output properties will actually save space compared to declaring additional (unique within the document) class strings. Worse, the browser has to do more work to apply the classes in sequence, rather than just having a single class worth of styles to apply—on top of which, minifiers can no longer help the browser by excising redundant declarations in advance.

Is there actually any significant net upside to including these? It seems like "just use mixins" is the best design, but I'd like to at least have an issue to discuss this in case I'm missing something important.

lukewestby commented 8 years ago

I wholeheartedly agree that @extend introduces more potential issues than it solves, so I'm -1 on including that type of composition. We've started using CSSModules at Raise and to me the biggest productivity gain there (aside from local scoping) is that it makes it much easier to manage conditional styles in React components. Instead of className={[classes.button, classes.buttonPrimary, classes.buttonLarge].join(' ')} we can just do className={classes.buttonPrimaryLarge}. Since elm-html is significantly more extensible that React's VDOM impl I think a helper named multiClass or something that combines classes and injects them into the className property on the VDOM node may solve the issue of composability without needing to add a construct like @extend or composes to the parsing process. Thoughts?

rtfeldman commented 8 years ago

Yeah, the way CSS Modules does that is what got me thinking about this. Certainly you can already use classList from elm-css-helpers to make reusable classes like buttonPrimaryLarge in that example. That's not quite what CSS Modules does, though, because with composes the composition is defined in the stylesheet at build time, not in the view at runtime.

So I thought "we already have mixins to compose styles at build time, but should we introduce something like composes as well?" This sounded like a red flag, because in that world, every time you want to compose something inside your stylesheet, you have to decide whether mixin or composes would be more appropriate. If only mixin exists, you just use that and move on.

This begged the question "if we already have mixin, under what circumstances would we wish we had composes?" I couldn't come up with a compelling answer to that. It seemed like introducing it would do more harm than good.

tl;dr Given that it doesn't have mixin, I think CSS Modules is better off sticking with composes as the single method of composition, and given that we already have mixin, I think we're probably better off sticking with that as the single method of composition.

rtfeldman commented 8 years ago

Okay, I'm happy with where this ended up. Closing!