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

Extending a compound selector violates extend semantics #1599

Closed
nex3 opened this Issue Jan 17, 2015 · 74 comments

Comments

Projects
None yet
@nex3
Contributor

nex3 commented Jan 17, 2015

Currently, when a compound selector is extended, Sass will only replace instances of the full extendee with the extender. For example:

.a {x: y}
.b {x: y}
.a.b {x: y}

.c {@extend .a.b}

produces

.a {x: y}
.b {x: y}
.a.b, .c {x: y}

when it should produce

.a, .c {x: y}
.b, .c {x: y}
.a.b, .c {x: y}

according to the semantics of @extend. We should fix this, especially as CSS moves closer to supporting @extend natively.

Tasks:

  • Deprecate existing behavior in stable.
  • Remove behavior from master.

@nex3 nex3 added this to the 4.0 milestone Jan 17, 2015

@lifeiscontent

This comment has been minimized.

lifeiscontent commented Mar 27, 2015

@nex3 any ideas when this might be fixed?

@chriseppstein

This comment has been minimized.

Member

chriseppstein commented Aug 31, 2015

To clarify:

.c { @extend .a.b } means "Style elements with class c, as if they had both of the classes .a and .b", It's semantically identical to .c { @extend .a; @extend .b; }

@ArmorDarks

This comment has been minimized.

ArmorDarks commented Dec 13, 2016

To be honest, I'm slightly confused.

This example seems to be perfectly right result for .c {@extend .a.b}:

.a {x: y}
.b {x: y}
.a.b, .c {x: y}

It's because we clearly asked to extend .a with specified by .b selector, not just .a or .b, or .a.b.

If we'd like to receive this:

.a, .c {x: y}
.b, .c {x: y}
.a.b, .c {x: y}

I'd more expect syntax like

.a {x: y}
.b {x: y}
.a.b {x: y}

.c {@extend .a, .b, .a.b}

or

.a {x: y}
.b {x: y}
.a.b {x: y}

.c {@extend .a.; @extend .b; @extend .a.b}
@nex3

This comment has been minimized.

Contributor

nex3 commented Dec 14, 2016

@ArmorDarks When you write A {@extend B}, that means "all elements that match A should be styled as though they also matched B". In the example above, that translates to "all elements that match .c should be styled as though they also matched .a.b". If an element matches .a.b, that means it also matches both .a and .b individually, so .c {@extend .a.b} should mean the same thing as .c {@extend .a; @extend .b}.

nex3 added a commit to sass/sass-spec that referenced this issue Jun 2, 2017

nex3 added a commit that referenced this issue Jun 2, 2017

nex3 added a commit that referenced this issue Jun 2, 2017

nex3 added a commit that referenced this issue Jun 2, 2017

nex3 added a commit to sass/sass-spec that referenced this issue Jun 2, 2017

nex3 added a commit that referenced this issue Jun 16, 2017

@nex3 nex3 closed this Jun 16, 2017

jhawthorn added a commit to jhawthorn/solidus that referenced this issue Jul 10, 2017

Don't @extend compound selectors in sass
The ability to extend compound selectors has been deprecated and will be
removed from sass.

See sass/sass#1599 for details.

jhawthorn added a commit to jhawthorn/solidus that referenced this issue Jul 11, 2017

Don't @extend compound selectors in sass
The ability to extend compound selectors has been deprecated and will be
removed from sass.

See sass/sass#1599 for details.
@ArmorDarks

This comment has been minimized.

ArmorDarks commented Jul 13, 2017

If an element matches .a.b, that means it also matches both .a and .b individually, so .c {@extend .a.b} should mean the same thing as .c {@extend .a; @extend .b}.

I still miss the logic why .a.b. also matches .a and .b individually. Those are 3 different selectors, it just so happens that they consist from same elements, but nothing more.

It just goes against CSS logic, since when in CSS we write more complex selectors, we increasing its specificity on purpose, to ensure that only "those instances of class" with class='a b' will receive that style, not class='a' and class='b'.

I think Sass can follow pretty much close same logic — when you increase specificity of extend, you want to extend only specific selectors, not everything related to them, otherwise you would write more generic extend.

@meowsus

This comment has been minimized.

meowsus commented Jul 14, 2017

@ArmorDarks that's the behavior I would expect. @extend should extend the selector that is passed to it. Anything more would risk yielding unexpected results, not to mention further tarnish it's already eroded reputation.

@mdesantis

This comment has been minimized.

mdesantis commented Jul 20, 2017

-1 to this. If I want .c to extend .a and .b I write .c { @extend .a; @extend .b; }.

@jdurand

This comment has been minimized.

jdurand commented Jul 26, 2017

I'm receiving deprecation warnings linking to this issue for several bits of SCSS that look like this:

.icon-check:before { content: "\f13e"; }

.foo {
  // [...]
  &:after {
    // [...]
    @extend .icon-check:before;
  }
}

This SCSS should probably be rewritten, but that's not the point; I don't think it's semantically wrong.
Why is it triggering this warning?

@Lepidora

This comment has been minimized.

Lepidora commented Feb 8, 2018

It annoys me that this (significant) change was made on the whim of someone writing a bug report. I feel for something with such widely used effects, it should be thoroughly discussed with the Sass developers and the community before any change is made.

I know I for one will be moving away from Sass in the future because I now have no idea what features could have their functionality arbitrarily changed.

@esr360

This comment has been minimized.

esr360 commented Mar 2, 2018

So....can we get some sort of update? Will anything be considered to cater for those of us who still need the old functionality? There have been some good suggestions so far...

@chriseppstein

This comment has been minimized.

Member

chriseppstein commented Mar 10, 2018

Hi everyone, I'd like to chime in here. For context, @extend was my idea, but when I initially proposed it, it only allowed extending simple selectors. As the design of the feature progressed, @nex3 realized that there was a more general definition that theoretically worked for compound selectors and she and I worked together to define the semantics of how that would work.

I want to be very clear about a few design goals of @extend. The primary design goal was to couple one class to another in CSS such that in markup, there was no longer a need to force a developer to apply several classes in conjunction. The definition of extend has always been about the net effect on markup.

To be honest, @extend was a huge experiment on our part, we knew that it had some gaps between the definition and the implementation (as noted in this thread, sometimes the cascade resolves incorrectly due to specificity issues) and we hoped that it would pave the way to a native implementation in CSS itself, which would not have those issues.

Historically, @extend has been the most problematic feature in Sass. So much so, that many projects use linting to forbid it from being used at all. Many developers do not grok how it should work and even when they do, they can still end up generating output that is far more complex than initially expected. With a heavy heart, when I defined the sass coding guidelines for linkedin, I too decided @extend was not safe for use on our styles at scale.

The complexity of this thread demonstrates the magnitude of how misunderstood the feature is by many developers. Much discussion has happened over the years to imagine ways of mitigating issues with extend.

I will point out that @nex3 has been a faithful maintainer of Sass for over a decade now. Natalie decided that porting to Dart was the path to creating a situation where she'd want to continue maintaining the project instead of simply abandoning the ruby implementation.

She and I decided, together, that the cost of porting a very complex feature with known issues (which she has patiently tried to explain in this thread) was not worth it. And, yes, we realized that this would affect users. First and foremost: you don't have to upgrade. Lock down your sass version in your Gemfile. There's no more new features coming to the ruby implementation, so you're not missing out on anything super important. Downgrade to before the deprecation and maintain your application's current working order. You were given free software that worked for you and it continues to do so at the version you had. The deprecation warnings are meant as a path to help ruby users port their stylesheets to the dart implementation.

The Dart implementation will not be getting an @extends that works as it did in ruby. The decision to reduce the surface area of this misunderstood aspect of the feature is a good one for the long term. And @nex3 went out of her way to add capabilities to the ruby implementation to detect and warn about this incapability -- That is a responsible and appropriate way to remove a feature. You're welcome. If you're decision is to stop using Sass because of this, so be it. But I will say that Sass has a decade of history of providing robust and well thought out designs. If you think it's operating on whims, you are very mistaken. Our record speaks for itself. Removing this feature that is quite misunderstood actually speaks volumes to the degree to which, we believe that the features Sass provides should be intuitive and robust against developers who do not fully understand implementation details and have not fully read the documentation we provide.

If you're using this feature and you want to port to Dart Sass, you have the following options:

  1. Extract the styles to a placeholder and extend it in both the original location of those styles and in the location where you used to extend the compound selector.
  2. Similar to above, you can extract a mixin and include it into both locations.
  3. You can remove your extend statement altogether and update your markup to match the original styles in conjunction with the additional styles it matched.

We understand that this is work and that it's annoying to do. We understand you're probably not happy about it and that you may be frustrated with this decision. As stated above, you only need to do this work IF you intend to port to Dart Sass. That's not a simple task, it will affect your build scripts, your app's code, and you should allocate an appropriate amount of time to do the work in accordance with your business's priorities.

@esr360

This comment has been minimized.

esr360 commented Mar 10, 2018

Disappointing news. Technology should evolve to cater for the needs of the users, not the other way around. But you're absolutely right, we get to use Sass for free, so far be it from us to complain. Sass is fantastic with or without this feature.

@jdurand

This comment has been minimized.

jdurand commented Mar 10, 2018

@esr360 this is free and open source software. Feel free to fork and maintain.

@esr360

This comment has been minimized.

esr360 commented Mar 10, 2018

@jdurand that's a very impractical solution and not how open source should really work.

@efojs

This comment has been minimized.

efojs commented Apr 6, 2018

@nex3, thank you for your patient explanation, and @chriseppstein for your long-read, and for Sass.

Extract the styles to a placeholder and extend it in both the original location of those styles and in the location where you used to extend the compound selector.

— this is what (who tbh did not read docs) I was looking for — solution for my hovered bar
.bar:hover { .button { @extend .a:hover; }}, in same case as @ArmorDarks mentioned:

It was useful, when element in unhovered state had to look like exactly as another hovered element.

You are doing great job!

@binki

This comment has been minimized.

binki commented Apr 9, 2018

I find myself agreeing with the logical argument of the OP.

However, I had written a bunch of code relying on the old behavior. Because I have control over my project, I am simply adding new placeholders next to each compound selector I want to be able to extend. I think this is the easiest way to transform massive amounts of existing code as it doesn’t require one to to understand how it works to port to sass-4:

Original (order changed so that specificity plays a role in the output, that @extend .a; @extend .b would result in an unwanted change in behavior):

.a.b {x: c}
.a {x: a}
.b {x: b}

.c {@extend .a.b}

On sass-4-pre, this is an error. The output of dart-sass:

Error: expected selector.
.c {@extend .a.b}
              ^
  - 5:15  root stylesheet

Modified:

%a-b, .a.b {x: c}
.a {x: a}
.b {x: b}

.c {@extend %a-b}

Output (manually slightly compressed):

.c, .a.b { x: c; }
.a { x: a; }
.b { x: b; }

The downside of this is that you have to remember to add the % selector to every instance of the particular compound selector you’re targeting. You cannot simply do a text search because SASS-3 at least knew that extending .a.b would also match .b.a and .a.c.b, for example.

@esr360

This comment has been minimized.

esr360 commented Jun 18, 2018

Sorry to revive this but there are still use-cases in these comments that are unaddressed and suggestions which have been ignored and I still require the ability to extend compound selectors and have so far not been convinced that my use-case is not legitimate.

Rather than ignoring the users who still require this functionality, could the developers instead try to understand our position and come up with some solution or better educate us on why our use cases are not legitimate so we no longer seek a solution?

As @binki mentioned, the only apparent way to refactor my code to accommodate this change is to move all my compound selector styles into a placeholder.

Whilst this is fine, it still seems like a step which should be unnecessary for me to take.

lamcw added a commit to lamcw/lamcw.github.io that referenced this issue Jun 26, 2018

Removed deprecated css selector
From Jekyll build warning:
DEPRECATION WARNING on line 219 of /home/lamcw/dev/lamcw.github.io/_sass/_syntax.scss:
Extending a compound selector, div.highlight, is deprecated and will not be supported in a future release.
Consider "@extend div, .highlight" instead.
See sass/sass#1599 for details.

Signed-off-by: lamcw <thomas@lamcw.com>

JoachimTillessen added a commit to emjot/active_bootstrap_skin that referenced this issue Jul 9, 2018

Fix Sass deprecation warning
Extending a compound selector, .btn-default.active, is deprecated and will not be supported in a future release.
Consider "@extend .btn-default, .active" instead.
See sass/sass#1599 for details.

butterflyhug added a commit to butterflyhug/sass that referenced this issue Jul 30, 2018

Doc: @extends complex selector deprecation
Update the Sass reference documentation to account for the decision
to remove support for `@extends` with complex selectors.

Per sass#1599 (comment),
sass/libsass#2418, and related issues.

butterflyhug added a commit to butterflyhug/sass that referenced this issue Jul 30, 2018

Doc: @extends complex selector deprecation
Update the Sass reference documentation to account for the decision
to remove support for `@extends` with complex selectors.

Per sass#1599 (comment),
sass/libsass#2418, and related issues.
@tassiogoncalves

This comment has been minimized.

tassiogoncalves commented Aug 16, 2018

Please, help me, this Dart Sass version not work @extend .class1, .class2;
Necessary coding @extend .class1; @extend class2;

How to proceed? This is correct? This (@extend .class1, .class2; ) is deprecated?

@nex3

This comment has been minimized.

Contributor

nex3 commented Aug 16, 2018

@tassiogoncalves @extend .class1, .class2 should work in Dart Sass. Can you file an issue?

@tassiogoncalves

This comment has been minimized.

tassiogoncalves commented Aug 16, 2018

n-b added a commit to betagouv/reso that referenced this issue Aug 29, 2018

Fix a sass deprecation warning
-> sass/sass#1599

The way we use it doesn’t seem to make any difference. 🤷‍♀️

n-b added a commit to betagouv/reso that referenced this issue Aug 29, 2018

Fix a sass deprecation warning
-> sass/sass#1599

The way we use it doesn’t seem to make any difference. 🤷‍♀️

n-b added a commit to betagouv/reso that referenced this issue Aug 29, 2018

Fix a sass deprecation warning
-> sass/sass#1599

The way we use it doesn’t seem to make any difference. 🤷‍♀️

vinhnglx added a commit to vinhnglx/active_bootstrap_skin that referenced this issue Sep 24, 2018

Fix Sass deprecation warning (#28)
Extending a compound selector, .btn-default.active, is deprecated and will not be supported in a future release.
Consider "@extend .btn-default, .active" instead.
See sass/sass#1599 for details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment