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

Deprecate extending compound selectors. #2300

Merged
merged 1 commit into from
Jun 2, 2017
Merged

Deprecate extending compound selectors. #2300

merged 1 commit into from
Jun 2, 2017

Conversation

nex3
Copy link
Contributor

@nex3 nex3 commented Jun 2, 2017

@nex3 nex3 merged commit f603639 into stable Jun 2, 2017
@nex3 nex3 deleted the no-extend-compound branch June 2, 2017 07:33
@spohlenz
Copy link

I came across this change today whilst upgrading to Sass 3.4.25, and I'm a little concerned.

A pattern that I have frequently used in the past is to add font icons before or after an element via CSS using @extend and the :before/:after selectors.

For example:

.sort-link:after {
  @extend .ion-chevron-down:before;
}

In 3.4.25, this code now emits a deprecation warning and the behavior will presumably be removed at some point.

So far I haven't been able to think of any alternative way of achieving this (short of putting the icon in the markup which I'd rather not have to do, particularly in cases where the icon can change according to the parent element's class).

Should :before and :after possibly be exceptions to this deprecation?

@meowsus
Copy link

meowsus commented Jul 11, 2017

When using icon fonts in my projects I roll the functionality into a mixin, to be as declarative as possible:

@include icon(name, placement, ...);

Which pulls the icon from a sass map:

$icons: (
  prev: '\e000',
  next: '\e001',
  ...
);

And drops it where it needs to be placed:

# scss
@mixin icon(name, placement) {
  &:#{placement} {
    content: map-get($icons, name);
  }
}

.component {
  @include icon(next, before);
}

# css
.component:before {
  content: '\e001';
}

this is just an example, your real mixin could accept many more arguments

The benefits to this approach are

  • the avoidance of extends
  • cleaner, more declarative code
  • futureproof

I'm a fan of extends. I know that there are lots of articles written about why you should never use them, but I do... sparingly... The biggest reason why I avoid them is because compiled CSS with repeatable patterns, like those given to you by frequent use of mixin, actually ends up smaller when your CSS assets are gzipped. It feels wrong, but the result is oh so right.

Just my two cents.

@spohlenz
Copy link

@meowsus That's definitely pretty clean, but in most cases I'm using a standard packaged icon font such as FontAwesome (usually using an asset gem such as font-awesome-rails), rather than my own custom font.

For those cases, I could potentially create my own map of the character codes, but then I have to go digging in the source code, and also run the risk of having them change on me during an update.

@nex3
Copy link
Contributor Author

nex3 commented Jul 12, 2017

There will be some use-cases that will be more difficult after that change, which is definitely unfortunate. There isn't necessarily a good drop-in replacement for the old behavior. But we feel strongly that it's important to have a clear and consistent set of semantics, and this behavior violates it. What's more, it's difficult for implementations to support this behavior, and removing it improves both performance and dev productivity.

Your specific example, :before, is particularly strange because it doesn't work like any other selector. When you write .foo:before, you're not referring to an element that matches both the selectors .foo and :before, you're referring to manufactured elements that exist relative to elements that match .foo. I suspect if this were created today, it would be ::before(.foo).

Because of this semantic difference, it would kind of make sense to allow @extend .foo:before for the same reason that @extend :matches(.foo, .bar) is allowed. But the syntactic difference between the two is relevant: we can process the latter as a single extended selector, but allowing the former would bring in all the complexity and performance issues that we're trying to avoid. I think ultimately it's not worth the cost.

@meowsus
Copy link

meowsus commented Jul 12, 2017

@spohlenz Ah, I see. At work a lot of our build teams use Font Custom via their rails gem, for better control over the icons the project uses.

At the beginning of a project, using an icon pack is great and extremely convenient. You never want to be tied down to creating and modifying icons when you're just trying to get the rubber to meet the road. Once the project reaches maturity, though, you're left with a large dependency which frequently goes underutilized. You find that, out of the hundreds of icons they provide, you're only using a small handful. That's not always the case, of course, but it's what I've seen time and time again.

So, do what thou wilt, but it sounds like it might be time to start hand crafting some artisanal, locally sourced, single origin, fair trade icons for your project.

Of course, if this is impossible/undesirable, I found this helpful gist.

@spohlenz
Copy link

@meowsus You're certainly not wrong, and that is something I definitely aim to do on my projects in general once a lot of the initial functionality and design is locked down. One of my major projects though is a framework within which I'd like developers to have access to all available icons, either via HTML tags or Sass.

For the cases mentioned (FontAwesome and IonIcons), I've been able to just use their original source repos, either via manually vendoring or via yarn/npm/bower. These font icons have the original character codes in Sass variables which I can easily use without fear of anything breaking in future.

However looking at the original issue #1599, I find myself agreeing more with @ArmorDarks, and disagreeing with @nex3 when she says "When you write A {@extend B}, that means "all elements that match A should be styled as though they also matched B"". My interpretation would be that "When you write A { @extend B }, that means 'extend the CSS selector matching B with A'".

Is there a canonical source for the original definition, or is this simply how the implementors wish it to be? Also, where would I find resources on how CSS is potentially going to be implementing @extend semantics? Closest thing I've found is https://tabatkins.github.io/specs/css-extend-rule/.

@nex3
Copy link
Contributor Author

nex3 commented Jul 13, 2017

Is there a canonical source for the original definition, or is this simply how the implementors wish it to be?

I don't think I'll be able to find the first place where I wrote that on the internet, but I can assure you that it's always been the stated semantics of @extend since I first created it.

Also, where would I find resources on how CSS is potentially going to be implementing @extend semantics? Closest thing I've found is https://tabatkins.github.io/specs/css-extend-rule/.

This is the closest thing that exists. This spec is based on Sass's semantics. Specifically, it says

An @extend rule modifies the way that selector matching works for the elements matched by the style rule the @extend selector is inside of, known as the extended elements for that rule.

The argument to @extend is the extension selector. The rule’s extended elements must, for the purpose of determining if selectors match them, act as if they had the necessary features/state/etc to match the extension selector, in addition to their pre-existing features/state/etc.

In other words, the extended elements are styled as though they also match the extension selector.

@ArmorDarks
Copy link

Ironically, I've wrote it #1599 (comment) and today I read in Sass 3.5 changelog exactly the opposite:

When you write h1 {@extend .a.b}, this should mean that all h1 elements are styled as though they match .a.b—that is, as though they have class="a b", which means they'd match both .a and .b separately. But instead we extend only selectors that contain both .a and .b, which is incorrect.

For sure I acknowledge that I might be wrong, but so far I can't get that logic. Though, I must be honest — I don't use @extend in projects very much and shouldn't care about it at all. But still would like to realize at last reasons behind this change.

@jhpratt
Copy link

jhpratt commented Jul 27, 2017

@nex3 Regarding ::before, would it be possible to allow it if there are two colons, rather than one? It's the proper way for pseudo-elements anyways.

@nex3
Copy link
Contributor Author

nex3 commented Jul 27, 2017

Unfortunately, no. If :before were newly introduced today, I would expect it to be written as ::before(.foo), which would work here. But I don't imagine that CSS is going to change such long-standing syntax.

@xtoq
Copy link

xtoq commented Dec 22, 2017

Hi there @nex3 I've read through both #1599 and this issue and I understand why the deprecation of extending compound selectors, and how that unfortunately impacts pseudo elements but I still have a (probably dumb) question.

Would it be possible/desirable to have pseudo elements in @extend behave like link states (:hover, :active, etc) when you extend an a tag? Or to have some sort of option or plugin to do so? Just wondering if there's a way to have my cake and eat it too! ;)

(Unrelated but thank you for your hard work on this amazing project.)

@nex3
Copy link
Contributor Author

nex3 commented Jan 3, 2018

@xtoq I'm not sure what you mean exactly. Can you give me an example?

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

Successfully merging this pull request may close these issues.

6 participants