Extended selectors wont compile correctly (with both regular and silent selectors) #146

Closed
lmartins opened this Issue Aug 9, 2013 · 112 comments

Projects

None yet
@lmartins
lmartins commented Aug 9, 2013 edited

When you have something like:

%btn-style-default{
  background: green;
  &:hover{
    background: black;
  }
}

and extend this selector with:

button{
  @extend %btn-style-default;
}

should compile to:

button{
   background: green;
}
button:hover{
   backgroud: black
}

but instead it compiles to invalid (and obviously non-functional) css:

button {
  background: green; }
  %btn-style-default:hover {
    background: black; }

Any news on this?

@lmartins
lmartins commented Aug 9, 2013

The only way I can get something similar is by using mixins, but that's far from ideal.

@hcatlin
Member
hcatlin commented Aug 9, 2013

Just added a spec test for this example...
sass/sass-spec@8a6a1c3

@akhleung
akhleung commented Aug 9, 2013

Referencing #80. This feature is difficult to implement completely, so I can't give an ETA yet.

@lmartins
lmartins commented Aug 9, 2013

@akhleung Sure, I get it. Thanks for all the hard work, wish I was able to help here.

@lmartins

Just to add to this issue, i've noticed that even regular selectors wont extend correctly:

.f-row--defaults{

  margin-bottom: 1.5em;

  &:last-child{
    margin-bottom: 0;
    border: 1px solid red;
  }

}

and then extend any selector with:
@extend .f-row--defaults;

It will only inerhit the base rule margin-bottom but not the child selector also declared on the selector used as extender.

@lmartins

Sorry to ping this, but is there any estimation on when will this be available?

@mshwery
mshwery commented Oct 21, 2013

+1. Really really incredibly important to the usefulness of sass.

@emagnier

+1. Same for me.

@JohnONolan

+1 this is a must-have

@ErisDS ErisDS referenced this issue in TryGhost/Ghost Oct 30, 2013
Closed

Remove dependency on Ruby #1346

@akhleung

We know this is an important feature and we're bumping up the priority on it ... it is, however, a very difficult feature to get completely right, but I'm hoping to consult with @nex3 in the near future to figure out how he did it in Ruby Sass.

@lmartins

You guys are doing an awesome job. Thank your for that and for tolerating our impatience.
I just can't use Ruby Sass now.

@benfrain

Just to echo everything Luis said :)

@benfrain
benfrain commented Nov 7, 2013

I've just added a bounty for this. I like to think it will buy those involved a beer/hot beverage/cocktail of choice whenever this gets finished. https://www.bountysource.com/issues/1057456-extend-classes-wont-compile-correctly-with-both-regular-and-silent-selectors/bounties

@emagnier
emagnier commented Dec 4, 2013

I also added a bounty for this one. Hope this will help :)

@akhleung
akhleung commented Dec 4, 2013

Working on the feature right now. 100% compatibility with Ruby Sass's @extend implementation is a monumental task, but I think I can at least address the specific use-cases in these tickets before the holidays.

@emagnier
emagnier commented Dec 4, 2013

Cool, that's great news! Thanks for all the hard work!

@dansowter

Really appreciate the hard work on this, guys. I'm porting over https://github.com/net-engine/trove to use libsass, and this seems to be one of the last issues.

@akhleung
akhleung commented Jan 6, 2014

Okay, the two examples given are now working correctly on the @extend branch, which I'll try to merge in a couple of days.

This is not to say that the feature is done -- far from it! I'm just trying to get it working one chunk at a time.

@jrabbe
jrabbe commented Jan 6, 2014

@akhleung is there a branch I can try to see if it works with our Sass?

@akhleung
akhleung commented Jan 6, 2014

The new code is in the @extend branch. It's a bit messy, but it you should be able to build it with the default makefile (I haven't updated the automake stuff yet).

@lmartins
lmartins commented Jan 7, 2014

Sorry to introduce some noise to the ticket comments but I just wanted to give thanks to @akhleung for the hard work.

@icasteleyn

Just wanted to add another example where @extends doesn't work properly.
".Sub.myInherit" should be ".myInherit.Sub"

Scss

.myBase { 
    text-decoration: none; 
}

.myInherit{
    @extend .myBase;
    line-height: 1.6;
}

.myBase.Sub{
    color: #123456;
}

SassLib

.myBase, .myInherit {
  text-decoration: none; }

.myInherit {
  line-height: 1.6; }

.myBase.Sub, .Sub.myInherit {
  color: #123456; }

Ruby Sass

.myBase, .myInherit {
  text-decoration: none; }

.myInherit {
  line-height: 1.6; }

.myBase.Sub, .myInherit.Sub {
  color: #123456; }
@akhleung
akhleung commented Jan 8, 2014

Interesting; I'll look into why the order gets mixed up. (At least the output is still semantically correct, if I'm not mistaken.)

@akhleung
akhleung commented Jan 8, 2014

Oh, also, I should mention that I've merged what I have so far into master, so @extend should work a little better than before.

@mgol
mgol commented Feb 3, 2014

Another test case (taken - and simplified - from our code base):

.common {
    color: red;
}
.outer {
    .inner {
        @extend .common;
    }
}
.outer2 {
    @extend .outer;
}

Correct output by Ruby Sass:

.common, .outer .inner, .outer2 .inner {
  color: red; }

libsass output:

.common, .outer .inner, .outer2 {
  color: red; }

The .inner part gets cut out.

@mgol
mgol commented Feb 3, 2014

And another, incorrect test case that gets properly shouted at by Ruby Sass:

.common {
    color: red;
}
.outer {
    .inner {
        @extend .common;
    }
}
.outer2 {
    @extend .middle;
}

(notice mismatched @extend .middle)

Ruby Sass gives:

Syntax error: ".outer2" failed to @extend ".middle".
              The selector ".middle" was not found.
              Use "@extend .middle !optional" if the extend should be able to fail.
        on line 10 of test.scss
  Use --trace for backtrace.

whereas libsass happily compiles that to the following:

.common, .outer .inner {
  color: red; }

Is there a proper place to report such issues? I have about 230 mismatched rules compared to the Ruby output, I can provide test cases but maybe it's not worth giving them all since many will be fixed when those already provided are fixed.

@akhleung
akhleung commented Feb 3, 2014

@mzgol @extend is still under active development, and I'm testing against the ~250 or so cases in the Ruby implementation, so it's probably not necessary for you to copy your broken cases here (unless you feel that it's something really unusual). Thanks!

@mgol
mgol commented Feb 3, 2014

@akhleung OK, thanks for the info! Do you think the implementation requires a lot more work? Not pushing, just curious if we have to wait a little longer.

Anyway, you're doing awesome job here, I bumped the bounty. :)

@akhleung
akhleung commented Feb 3, 2014

It's hard to say for sure, but I think @extend will still require a lot more effort -- that single feature has turned out to be much more sophisticated and subtle than I ever would have imagined when we started this project! Fortunately, @nex3 was kind enough to write up a description of his algorithm, and I've been reading the Ruby source as well, so I hope to get it all figured out in a month or two (depending on how much time I'm able to devote to this project, of course).

@anotheruiguy

If anyone is looking for a summary of issues I encountered with libsass' version of extends, please see this post.

akhleung has an amazing undertaking, we appreciate all your work in this regard.

@vaibhavkanwal

I switched to libsass for an existing enterprise project at work. All my @extends do not work. So, is there a workaround to get the speeds of libsass and work temporarily with extend till a good solution comes up.

@mgol
mgol commented Feb 12, 2014

@vaibhavkanwal Obviously not... If such a workaround existed it wouldn't really be a workaround but an implementation and it would be merged to libsass. Today you can choose between speed & reliable @extend.

@vaibhavkanwal

@mzgol Sure. Makes sense. Eagerly await this fix.

@akhleung

@vaibhavkanwal All of your @extends are breaking? I did push some new code a couple of months ago based on the algorithm in Ruby Sass -- perhaps that was premature. I'll see if I can make some improvements by early next week. Unfortunately, the full algorithm is complex enough such that doing it in bits and pieces won't produce reliable results; I need to port the whole thing, which I haven't had time for recently.

@vaibhavkanwal

@akhleung I understand, thanks! I use @extend with silent classes. It did not give error for me but did not compile the code in output so styles were not applied. Hope that helps.

@akhleung

Oh, also, if you could post a simple test case that is failing for you, that would be helpful. Thanks!

@dlmanning dlmanning referenced this issue in dlmanning/gulp-sass Feb 26, 2014
Closed

@extend doesn't work with nested selectors #29

@joeyhoer joeyhoer referenced this issue in dlmanning/gulp-sass Mar 4, 2014
Closed

@extend not working across imports #12

@mlmorg
mlmorg commented Mar 6, 2014

+1

@halfdan
halfdan commented Mar 11, 2014

@akhleung Here's another example that works fine in Ruby Sass but doesn't produce the desired result in libsass: http://sassmeister.com/gist/9482759 (you can switch the compile mode on the top right corner).

Is there any ETA on the support for @extend?

@mgol
mgol commented Apr 11, 2014

I kindly remind anyone interested in it that there's an open bounty:
https://www.bountysource.com/issues/1057456-extended-selectors-wont-compile-correctly-with-both-regular-and-silent-selectors-185
Currently $185, let's make it bigger! :-)

@lunelson

Some of these use-cases are already working, based on what I can see on sassmeister; but a modification of @anotheruiguy's example—which is still broken—produces an interesting glitch:

%new-parent {
  border-width: 1px;
  border-style: solid;
  &.background-color {
    background-color: orange;
  }
  &.add-border {
    border: 1px solid red;
  }
}
.outer-box {
  border-width: 1px;
  border-style: solid; }
  .background-color.outer-box {
    background-color: orange; }
  .add-border.outer-box {
    border: 1px solid red; }

The silent class is not repeated here; but the parent selector reference is coming out in reverse order. It would still be valid since a compound class is valid in any order, but I wonder if it points to something deeper?

Anyway basic implementation of silent/placeholder classes—notwithstanding classes nested in the placeholder—is working for me so thanks @akhleung and everyone else.

@am11 am11 referenced this issue in madskristensen/WebEssentials2013 Apr 23, 2014
Closed

Sass @extend doesn't extend as much as it should #769

@Ilyes512 Ilyes512 added a commit to Ilyes512/WPF that referenced this issue May 2, 2014
@Ilyes512 Ilyes512 Moved from grunt-sass to grunt-contrib-sass
One of the reasons for switching is: sass/libsass#146
a79fb22
@bonfish
bonfish commented May 21, 2014

Hi! I am experiencing an @extend error using grunt-sass. The SCSS:

.module {
  padding: 10px;
  h3 {
    color: red; 
  }
}

.news {
  @extend .module;
}

should produce the CSS

.module, .news {
  padding: 10px;
}
.module h3, .news h3 {
  color: red;
}

but instead, it gives:

.module, .news {
    padding: 10px;
}
.module h3, .module .news {
    color: #FF0000;
}

Do I get this wrong, or is the culprit really the libsass in grunt-sass ?

@hcatlin
Member
hcatlin commented May 21, 2014

Unfortunately, it's libsass doing that... ;/

On Wednesday, May 21, 2014, Aleksey notifications@github.com wrote:

Hi! I am experiencing an @extend https://github.com/extend error using
grunt-sass. The SCSS:

.module {
padding: 10px;
h3 {
color: red;
}
}

.news {
@extend .module;
}

should produce the CSS

.module, .news {
padding: 10px;
}
.module h3, .news h3 {
color: red;
}

but instead, it gives:

.module, .news {
padding: 10px;
}
.module h3, .module .news {
color: #FF0000;
}

Do I get this wrong, or is the culprit really the libsass in grunt-sass
?

Reply to this email directly or view it on GitHubhttps://github.com/hcatlin/libsass/issues/146#issuecomment-43721035
.

@bonfish
bonfish commented May 27, 2014

@hcatlin thanks, though I was hoping to get the opposite answer.

SASS is sweet, the ability to work with it in such side tools as Grunt helps a lot. Extremely eager to see an update =)

Meanwhile, thank you guys so much for your work, wish all the best to the team!
Aleksey.

@Cinamonas

@bonfish Sass itself has nothing to do with this issue. Until libsass is fixed, you can use slower grunt-contrib-sass, which runs on Ruby.

@KayLeung

@bonfish , or change your code a bit. libsass worked with non-nested @extend & %placeholder

@Ilyes512

@KayLeung I would rather use Ruby Sass, it just 2 seconds compilation at most?

I havent looked into the problem, but I find it strange that Ruby can do something that C cant? Ruby Sass is opensource right?

@lunelson

@Ilyes512 ruby sass gets much slower than that on big projects. I've seen cases with over 5 seconds compile time. Which is just not workable, in this era where we're told we should 'design in the browser'. Speed is the basic reason for libsass. See @benfrain's article for reference http://benfrain.com/libsass-lightning-fast-sass-compiler-ready-prime-time/; and yes as @KayLeung said you can just simplify your use-cases for @extend. Basic non-nested usage and placeholder classes are already working.

@hcatlin
Member
hcatlin commented May 28, 2014

Actually, Ruby Sass can take up to 2 hours to compile on very, very complex
projects, but several minutes on regular large projects. Libsass is a new
project to try and speed up Sass compilation, and ease both integration and
memory usage.

Feel free to use the main Ruby-based project if it better fits your needs.

On Wednesday, May 28, 2014, Lu Nelson notifications@github.com wrote:

@Ilyes512 https://github.com/Ilyes512 ruby sass gets much slower than
that on big projects. I've seen cases with over 5 seconds compile time.
Which is just not workable, in this era where we're told we should 'design
in the browser'. Speed is the basic reason for libsass. See @benfrainhttps://github.com/benfrain's
article for reference
http://benfrain.com/libsass-lightning-fast-sass-compiler-ready-prime-time/;
and yes as @KayLeung https://github.com/KayLeung said you can just
simplify your use-cases for @extend. Basic non-nested usage and
placeholder classes are already working.

Reply to this email directly or view it on GitHubhttps://github.com/hcatlin/libsass/issues/146#issuecomment-44396470
.

@cjcheshire

2 hours! Thank god i'm not on that project! :)

On 28 May 2014 16:31, Hampton Catlin notifications@github.com wrote:

Actually, Ruby Sass can take up to 2 hours to compile on very, very complex
projects, but several minutes on regular large projects. Libsass is a new
project to try and speed up Sass compilation, and ease both integration and
memory usage.

Feel free to use the main Ruby-based project if it better fits your needs.

On Wednesday, May 28, 2014, Lu Nelson notifications@github.com wrote:

@Ilyes512 https://github.com/Ilyes512 ruby sass gets much slower than

that on big projects. I've seen cases with over 5 seconds compile time.
Which is just not workable, in this era where we're told we should
'design
in the browser'. Speed is the basic reason for libsass. See @benfrain<
https://github.com/benfrain>'s

article for reference

http://benfrain.com/libsass-lightning-fast-sass-compiler-ready-prime-time/
;
and yes as @KayLeung https://github.com/KayLeung said you can just

simplify your use-cases for @extend. Basic non-nested usage and
placeholder classes are already working.

Reply to this email directly or view it on GitHub<
https://github.com/hcatlin/libsass/issues/146#issuecomment-44396470>

.


Reply to this email directly or view it on GitHubhttps://github.com/hcatlin/libsass/issues/146#issuecomment-44413944
.

@lmartins

2 hours might be extreme, but in my experience it is quite easy to get to 10sec average compilation times with ruby sass, which is unbearable to me. That is why I approach as @lunelson suggests, by being careful with some features and give for now on others.
To me personally, the speed alone more than compensates for the missing (for now) features.
Once you get used to have instant compilation times, you won't look back I promise.

@cjcheshire

We resorted to the gem to be safe but it is take a few seconds to compile
and then node reloading which does delay designing in the browser
On 28 May 2014 17:02, "Luis Martins" notifications@github.com wrote:

2 hours might be extreme, but in my experience it is quite easy to get to
10sec average compilation times with ruby sass, which is unbearable to me.
That is why I approach as @lunelson https://github.com/lunelsonsuggests, by being careful with some features and give for now on others.
To me personally, the speed alone more than compensates for the missing
(for now) features.
Once you get used to have instant compilation times, you won't look back I
promise.


Reply to this email directly or view it on GitHubhttps://github.com/hcatlin/libsass/issues/146#issuecomment-44418350
.

@lbastendorff

20 minutes of our 45 minute build time is down to ruby sass. Still hope we can get libsass to do the compass work oneday. Long term goal is to be able to recompile the sass based on variables customisable on the front end. Then the user would save variables relating to font styles and background colours in the web gui and the sass would need to compile in a much more sensible time frame and take effect within the application.

@restlessdesign

Build times and variables have nothing to do with this ticket. Keep the discussion on-topic please!

@lmartins lmartins changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$185] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,185] Jun 6, 2014
@lmartins lmartins added the bounty label Jun 6, 2014
@Westbrook

@hcatlin, @akhleung, really interested in getting this worked out, so I just bumped up the bounty. If either of you know more about the time to complete, would be willing to discuss what you think might be a more appropriate number.

@lmartins lmartins changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,185] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,190] Jun 6, 2014
@lmartins lmartins added the bounty label Jun 6, 2014
@lmartins lmartins changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,190] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,205] Jun 6, 2014
@lmartins lmartins added the bounty label Jun 6, 2014
@hcatlin
Member
hcatlin commented Jun 7, 2014

Yeah, wowza! I saw that happen and nearly dropped my coffee. That's really,
extremely generous of you.

The complicated part is that the projects are partially sponsored by
companies. Aaron is paid as part of his
job (sometimes) to work on libsass. However, since weekend time isn't
really sponsored and I don't think
Aaron has enough time during the workweek to get such a large feature
finished, this seems like great incentive. ;)

OR, anyone else out there who wants a little extra money can take a stab at
tackling this!

On Fri, Jun 6, 2014 at 3:47 PM, Westbrook notifications@github.com wrote:

@hcatlin https://github.com/hcatlin, @akhleung
https://github.com/akhleung, really interested in getting this worked
out, so I just bumped up the bounty. If either of you know more about the
time to complete, would be willing to discuss what you think might be a
more appropriate number.

Reply to this email directly or view it on GitHub
#146 (comment).

@akhleung
akhleung commented Jun 7, 2014

+1 to what Hampton said, and I'll add that because my employer pays me to work on LibSass (sometimes), what really means a lot to us here isn't the money itself, but rather the show of serious support. This sends a very clear signal that people are taking the project seriously, and for that you have my sincerest thanks.

@Westbrook

It all stems from the great work yall are doing. LibSass makes a huge
difference to our stack and the features we want to deliver on it, so we're
really looking forward to having it in a place where we can deploy it to
production. Thanks again!

On Sat, Jun 7, 2014 at 11:30 AM, Aaron Leung notifications@github.com
wrote:

+1 to what Hampton said, and I'll add that because my employer pays me to
work on LibSass (sometimes), what really means a lot to us here isn't the
money itself, but rather the show of serious support. This sends a very
clear signal that people are taking the project seriously, and for that you
have my sincerest thanks.


Reply to this email directly or view it on GitHub
#146 (comment).

@hcatlin
Member
hcatlin commented Jun 9, 2014

@Westbrook email me at hcatlin@gmail.com

On Sat, Jun 7, 2014 at 5:02 PM, Westbrook notifications@github.com wrote:

It all stems from the great work yall are doing. LibSass makes a huge
difference to our stack and the features we want to deliver on it, so
we're
really looking forward to having it in a place where we can deploy it to
production. Thanks again!

On Sat, Jun 7, 2014 at 11:30 AM, Aaron Leung notifications@github.com
wrote:

+1 to what Hampton said, and I'll add that because my employer pays me
to
work on LibSass (sometimes), what really means a lot to us here isn't
the
money itself, but rather the show of serious support. This sends a very
clear signal that people are taking the project seriously, and for that
you
have my sincerest thanks.

Reply to this email directly or view it on GitHub
#146 (comment).

Reply to this email directly or view it on GitHub
#146 (comment).

@andrewdc
andrewdc commented Jun 9, 2014

Yes, I would like to put a word in for how important this project is, and
that it will hugely benefit our team to have this particular feature
working. We implement a hybrid oocss + smacss + bem approach to our front
end and @extend is a huge aspect of it. It has changed the way we do
things. I understand @extend is very complicated, so I wanted to thank
everyone involved. Using gulp.js I saw a compilation speed increase that
was magnitudes quicker than the ruby-sass equivalent, and we all look
forward to making the switch to libsass once extends and maps are 100%.
Thanks everyone.

@epitaphmike epitaphmike referenced this issue in koistya/gulp-csscomb Jun 10, 2014
Closed

SCSS @media variable breaks CSSComb #3

@lmartins lmartins changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,205] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,230] Jun 15, 2014
@lmartins lmartins added the bounty label Jun 15, 2014
@krisbulman

Added to the bounty, great work guys.

@jaddie
jaddie commented Jun 22, 2014

Hey was just wondering how this is going, as I am encountering an error and wondering if it is caused as a result of this pending work? expand.cpp:264 "error("nested selectors may not be extended", c->path(), c->position(), backtrace);"

Did try a tweet but thought I should post here too :)

@lmartins lmartins changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,230] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,250] Jun 24, 2014
@lmartins lmartins added the bounty label Jun 24, 2014
@nchafai
nchafai commented Aug 7, 2014

Hi everyone,

Although I know @akhleung has huge work to resolve this issue, does anyone know how to adapt our code in order to dodge these @extend issues ?

I mean, Ruby-Sass is currently taking about 30sec to 1min on my (slow) computer so that I do have to use libsass. But the generated CSS is completely wrong due to these @extend.

Is there any workaround ?

@benfrain
benfrain commented Aug 7, 2014

It would probably make sense to create a codepen, sassmeister or gist with your existing Ruby based Sass code. There are different approaches that can work around some of the remaining extend issues.

On 7 Aug 2014, at 10:51, nchafai notifications@github.com wrote:

Hi everyone,

Although I know @akhleung has huge work to resolve this issue, does anyone know how to adapt our code in order to dodge these @extend issues ?

I mean, Ruby-Sass is currently taking about 30sec to 1min on my (slow) computer so that I do have to use libsass. But the generated CSS is completely wrong due to these @extend.

Is there any workaround ?


Reply to this email directly or view it on GitHub.

@lmartins
lmartins commented Aug 7, 2014

@nchafai In my personal experience, and this is only my opinion, the time saved with libsass more than compensates for the extra work for avoiding @extend. I get this is not always feasible, and in an ideal world libsass would be in feature parity with ruby sass, but that isn't the case and up until then you will most likely have to make a choice.

Again, for me that meant going for simpler sass code and avoiding some libraries. I am quite happy with that choice, and would do it again today.

@nchafai
nchafai commented Aug 7, 2014

@benfrain Unfortunately, the state of the project is that I implemented scss files with multiple @extend based on sass libraries (smartadmin, bootstrap, fontawesome, and a project library) using Ruby-Sass preprocessor. Switching to libsass, I do have to find which are the faulty @extend before debugging. I might post these cases as soon as I would have find them :)

@lmartins Avoiding some libraries is not a workaround in my case (otherwise, I would have to stop using SASS!).

Maybe writing simpler sass code (when extending my own classes, not library-ones) is a good directive, but i sometime feels like i will write close-to-css code.

And for library-classes, i will sometimes have to specify them directly in the DOM, losing the usefullness of SASS.

Anyway, thanks for the feedback. I'll try to apply the "simpler sass code" directive and see what happens !

@michaek
Contributor
michaek commented Aug 8, 2014

I'm curious what would be accepted as closing this issue. Passing all the extend-tests in https://github.com/sass/sass-spec, some subset of those tests, or some other criteria?

@akhleung
akhleung commented Aug 8, 2014

Given the size of the bounty and the complexity of the reference implementation (which makes it hard to port over in an incremental manner), I believe that passing all the extend-tests is a reasonable requirement.

@michaek
Contributor
michaek commented Aug 8, 2014

Yep, that makes sense to me. I've been coming back to this issue periodically over the past year, and it seemed helpful to have the criteria stated clearly. :)

@Westbrook

I'm definitely hoping that the interest in the bounty that my contributions have/can raise will go to the betterment of the community at large, and a such a pass for all the extend-tests.

@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,350] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,385] Aug 11, 2014
@hcatlin hcatlin added the bounty label Aug 11, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,385] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,400] Aug 22, 2014
@hcatlin hcatlin added the bounty label Aug 22, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,400] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,500] Aug 22, 2014
@hcatlin hcatlin added the bounty label Aug 22, 2014
@rryter
rryter commented Aug 22, 2014

I also added a little bit to the bounty. To all involved in this: thank you so much for your effort!
We have just switched to libsass and dropped the Ruby on Rails asset-pipeline. It is really boosting our development speed... keep up the good work!

@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,500] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,510] Aug 22, 2014
@hcatlin hcatlin added the bounty label Aug 22, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,510] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,565] Aug 27, 2014
@hcatlin hcatlin added the bounty label Aug 27, 2014
@sheerun
sheerun commented Aug 27, 2014

@akhleung Could you describe your approach to fixing this issue, if you have any?
What code is broken, what refactor could possibly help, what needs to be done at high level?

@fourseven fourseven referenced this issue in fourseven/meteor-scss Aug 28, 2014
Closed

Extend class with pseudo elements not working #14

@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,565] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,815] Sep 5, 2014
@hcatlin hcatlin added the bounty label Sep 5, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,815] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,865] Sep 5, 2014
@hcatlin hcatlin added the bounty label Sep 5, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,865] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,880] Sep 6, 2014
@hcatlin hcatlin added the bounty label Sep 6, 2014
@michaek
Contributor
michaek commented Sep 9, 2014

@sheerun Sorry I missed your comment until now!

The main problem is that @extend is a very complex feature, even if it doesn't seem that way when using it. See @nex3's "How @extend works" for a partial description of the algorithm.

The problem has two major parts:

  1. Most of the @extend algorithm hadn't been ported yet from Ruby
  2. The AST data model in libsass doesn't exactly match that of Ruby Sass, which adds to the complexity of the port

Currently, my team (primarily @uberska) is chipping away at those issues. We're working on a couple branches in a fork of libsass, implementing first the weave function then the subweave function. Simultaneously, we're triaging the tests in https://github.com/sass/sass-spec to determine which currently failing tests are related to @extend functionality, and which are failing because of other concerns such as AST parsing.

We can't commit to getting all @extend tests passing, but we're working hard on covering common cases - which include the complex weave/subweave behavior.

@jrabbe
jrabbe commented Sep 10, 2014

Thanks for chiming in @michaek, I know it's not necessarily the correct place for it, but I was wondering if you could weigh in on the difference between the data models in libsass and Ruby Sass, and why they are different? Thanks a bunch.

@michaek
Contributor
michaek commented Sep 11, 2014

@jrabbe The main difference we ran into is that when storing complex selectors, Ruby SASS stores combinators and compound selectors in separate array positions: [a, >, b, c], while libsass combines each combinator with its following combinator into a ComplexSelector object [[a, >], [b, " "], [c, " "]]. That ends up being relevant in the implementations of subweave/lcs/trim, so it made sense for us to use a structure that allowed our code to be a clearer port of the Ruby implementation.

@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,880] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,930] Sep 12, 2014
@hcatlin hcatlin added the bounty label Sep 12, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,930] to Extended selectors wont compile correctly (with both regular and silent selectors) [$1,985] Sep 13, 2014
@hcatlin hcatlin added the bounty label Sep 13, 2014
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$1,985] to Extended selectors wont compile correctly (with both regular and silent selectors) [$2,000] Sep 14, 2014
@hcatlin hcatlin added the bounty label Sep 14, 2014
@michaek
Contributor
michaek commented Sep 18, 2014

Our current work on @extend is now passing all but 55 tests. Here are the remaining tests: https://github.com/DealerDotCom/sass-spec/tree/master/spec/todo/extend-tests Because they're mostly esoteric (aside from 090_test_comma_extendee) my assumption is that these are not dealbreakers for most people currently using @extend.

That said, we're evaluating whether we can squash the remaining 55!

@Westbrook

Great news @michaek!! Do you have any idea when people outside of your time might be able to review your fork/branch of the code? I'd love to confirm it against the specific use cases with which my team and I are working.

@michaek
Contributor
michaek commented Sep 18, 2014
@lunelson

I discovered an odd thing with chaining of extends, which isn't incorrect but unexpected. In the following example if I extend %orig directly the selectors come out the way one would expect i.e. .h1.single, h1.single, but if I extend the %inter declaration, the first compound class gets the class-names reversed.
Just posting in case it's of interest to those working on the feature and running tests.

%orig {
  &.single {
    line-height: 1;
    margin-top: 0;
    margin-bottom: 0;
  }
}

%inter {
  @extend %orig;
}

.h1, h1 {
  @extend %inter;
}
.single.h1, h1.single {
  line-height: 1;
  margin-top: 0;
  margin-bottom: 0; }
@michaek
Contributor
michaek commented Sep 23, 2014

@lunelson Which libsass are you testing with? The current from https://github.com/sass/libsass or the work in progress from https://github.com/DealerDotCom/libsass/tree/feature/extend-support?

The current libsass shouldn't be expected to work for any but the simplest cases, and I get the same output as Ruby Sass from the feature/extend-support code.

For housekeeping, can we avoid posting further cases with @extend on this issue? The cases in https://github.com/sass/sass-spec are pretty exhaustive. If you can't find your case there, you can make a pull request to add it. Thanks!

@lunelson

I've got the official one. No problem, I'll be quiet ;)

@hcatlin hcatlin pushed a commit to sass/sass-spec that referenced this issue Oct 3, 2014
Hampton Catlin adding in the original extends spec test from the $2k bounty issue. i…
…t now passes on libsass! sass/libsass#146
1be9aa6
@hcatlin
Member
hcatlin commented Oct 3, 2014

A year after its opening... I'm ridiculously, incredibly excited to click the close button on this issue. Thanks to @uberska @jaddessi and @michaek this mega-extends-issue can now be closed. The original test case now passes, as do many, many, many, many other new ones written by the team at @dealerdotcom. Also, super thanks to @paulirish for funding the bounty on this and never letting us forget that this was pending. So, @uberska what are you going to do with the bounty? ;)

@hcatlin hcatlin closed this Oct 3, 2014
@hcatlin hcatlin added the bounty label Oct 3, 2014
@lmartins
lmartins commented Oct 3, 2014

Thank you all for amazing effort.

@benfrain
benfrain commented Oct 3, 2014

Brilliant. Just checked my mail and I opened the bounty on the 7th Nov 2013! It sounded like a Herculean task so Its great to see it finally closed after all these months. Great work all involved.

Can't wait to use it in the wild :)

On 3 Oct 2014, at 18:22, Hampton Catlin notifications@github.com wrote:

A year after its opening... I'm ridiculously, incredibly excited to click the close button on this issue. Thanks to @uberska @jaddessi and @michaek this mega-extends-issue can now be closed. The original test case now passes, as do many, many, many, many other new ones written by the team at @dealerdotcom. Also, super thanks to @paulirish for funding the bounty on this and never letting us forget that this was pending. So, @uberska what are you going to do with the bounty? ;)


Reply to this email directly or view it on GitHub.

@andrewdc
andrewdc commented Oct 3, 2014

Fantastic news. This issue was a major blocker for a smacss/bem approach
our team is using. Thanks to everyone who worked on this.
On Oct 3, 2014 10:22 AM, "Hampton Catlin" notifications@github.com wrote:

A year after its opening... I'm ridiculously, incredibly excited to click
the close button on this issue. Thanks to @uberska
https://github.com/uberska @jaddessi https://github.com/jaddessi and
@michaek https://github.com/michaek this mega-extends-issue can now be
closed. The original test case now passes, as do many, many, many, many
other new ones written by the team at @dealerdotcom
https://github.com/dealerdotcom. Also, super thanks to @paulirish
https://github.com/paulirish for funding the bounty on this and never
letting us forget that this was pending. So, @uberska
https://github.com/uberska what are you going to do with the bounty? ;)


Reply to this email directly or view it on GitHub
#146 (comment).

@blackfalcon

Congrats to everyone all around! This is the final step we needed to move forward and have the community build more trust in this version of the language.

libsass is ready for primetime! I am calling it!

@paulirish
Member

Huge thanks to all the supporters on Bountysource! $2000 was raised for this feature! whoa.
Thanks to photoshelter.com via @Westbrook who contributed $1000 of that!

And big ups to @DealerDotCom who sponsored @uberska's development time on the project. Awesome work, everyone.

@Westbrook

A bit of cheer went up around the office today when this came in. Many thanks @uberaka and @DealerDotCom for making this a reality.

We're super stoked for the possibilities this opens up for the LibSass community at large and are already working to get this into our stack so we can see it in our production code as soon as possible!

On Oct 3, 2014, at 5:07 PM, Paul Irish notifications@github.com wrote:

Huge thanks to all the supporters on Bountysource! $2000 was raised for this feature! whoa.
Thanks to photoshelter.com via @Westbrook who contributed $1000 of that!

And big ups to @DealerDotCom who sponsored @uberska's development time on the project. Awesome work,e veryone.


Reply to this email directly or view it on GitHub.

@thisguychris

"That said, we're evaluating whether we can squash the remaining 55!"

@michaek Thank you for the brilliant solution! So I just want to ask, did the remaining 55 tests got squashed?

@albell
albell commented Oct 3, 2014

Huge thanks to everyone on this. I anticipate that this will cut my day-to-day sit-and-wait-for-compile time massively. Highly appreciated.

@restlessdesign

Huge! Thanks to all who helped on this! 🎉 🎉 🎉

@jory
jory commented Oct 3, 2014

I'd like to add: WOOOOOOOOOOOOOO!

@michaek
Contributor
michaek commented Oct 4, 2014

@deezahyn There are now 35 extend tests that fail/error, but they're all rare edge cases, with the exception of 090_test_comma_extendee. That's the syntax that allows @extend a, b; as a shorthand for @extend a; @extend b;.

And please open new issues for those failing tests that turn out to be dealbreakers for you!

@bonfish
bonfish commented Oct 4, 2014

Great news! Looking forward for the new version out, making it possible for me to move back from Ruby SASS.

One thing I gon't quite get. @uberska who earned all the praise as a developer - is not part of the Libsass team? And...

And big ups to @DealerDotCom who sponsored @uberska's development time on the project.

... means that this huge task was backed not only by one of the Top three biggest bounties on Bountysource?

@sintaxi
sintaxi commented Oct 5, 2014

This is wonderful. Thanks to all who helped make this happen.

+1 to EOL the Ruby implementation.

@jschulte
jschulte commented Oct 5, 2014

@bonfish Good questions! @DealerDotCom sponsored the development work on this (devoted full time employees to work on it for weeks!) because it is valuable for our development process efficiency. @uberska and @jaddessi had a lot of prior C++ experience, so they did the lion's share of the development work. @michaek and I helped with writing tests, fixing a few bugs and validating their work. It was a really fun project to work on and we're thrilled that it's available for others to enjoy!

@bonfish
bonfish commented Oct 5, 2014

@jschulte that sounds very inspiring! Thank you so much for your work!

One more question, if I may: is there a list of tests somewhere in the public, that at-extend is still uncapable of passing? Just to get the view of what constructs to evade in advance.

@renaudleo

<3

@antoinelyset

🎉

@dilrajahdan

Excellent Work!

@pkyeck
pkyeck commented Oct 15, 2014

thanks for the hard work. 🎉

@hcatlin which commit closed this issue? I'm still getting wrong output from node-sass although they updated their dependencies some days ago ...

@michaek
Contributor
michaek commented Oct 16, 2014

The node-sass project will be doing a release once libsass has its 3.0.0 release. In the meantime, it is possible to build node-sass (and thus use it) following the instructions here: https://github.com/sass/node-sass#rebuilding-binaries That said, it's not exactly for the faint of heart!

@michaek
Contributor
michaek commented Oct 17, 2014

These are not all relevant to everyone, but the work on this issue has now been released where most consumers can "just use it":

libsass 3.0.0 is released
node-sass 1.0.0 is released
grunt-sass 0.16.0 is released
gulp-sass 1.2.0 is released

@paulirish
Member

Impressed with the coordination across projects such they all release the same day! Nice work to @hcatlin and all the maintainers for making this nice for all users. :D

@davecranwell davecranwell referenced this issue in wagtail/wagtail Oct 22, 2014
Closed

Update version of sassc/libsass used #271

@sandstrom

Awesome!! ⛵️

@gorilas
gorilas commented Nov 11, 2014

It has been amazing what happened!!!! Thanks to everyone. Hurra y Viva!!!!

@jbeja
jbeja commented Nov 13, 2014

Omg, I can't believe this is finally solve. Thanks to everyone from the bottom of my heart <3.

@valdelama valdelama referenced this issue in dlmanning/gulp-sass Nov 21, 2014
Closed

Update Node Sass dependency #139

@ldexterldesign ldexterldesign referenced this issue in zurb/foundation-libsass-template Dec 3, 2014
Closed

Update package.json #13

@ldexterldesign

Apologies for the noise folks, but hoping someone will more knowledge can help me?:
https://github.com/sindresorhus/grunt-sass/issues/162

Yours hopefully,

@dreamyguy

Just had to say my thanks to this fix! Life-saver, just in time! ❤️

@quis quis referenced this issue in alphagov/govuk_frontend_toolkit Dec 8, 2014
Merged

Add note to readme about incompatible Libsass versions #153

@FMCorz FMCorz referenced this issue in driftyco/ionic-cli Feb 3, 2015
Closed

Sass does not support @extend #217

@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$2,000] to Extended selectors wont compile correctly (with both regular and silent selectors) [$2,000 awarded] Feb 21, 2015
@downzer0 downzer0 referenced this issue in edx/ux-pattern-library Jun 15, 2015
Closed

Elements: Forms and form controls #75

@mgreter mgreter removed the bounty label Oct 22, 2016
@hcatlin hcatlin added the bounty label Dec 29, 2016
@hcatlin hcatlin changed the title from Extended selectors wont compile correctly (with both regular and silent selectors) [$2,000 awarded] to Extended selectors wont compile correctly (with both regular and silent selectors) Jan 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment