Extending placeholders which have nested media queries leaks placeholder into CSS output

[keithamus]

Abstract

Using a placeholder selector, calling a mixin which wraps a media-query, the placeholder selector is leaked into the CSS output.

Detail

Given the following SCSS:

%foo {
    @media screen and (min-width: 300px) {
        max-width: 80%;
    }
}

body {
    @extend %foo;
}

Expected

The output of sass (3.3.1 and 3.2.15)

@media screen and (min-width: 300px) {
  body {
    max-width: 80%; } }

Actual

The output of libsass (actually, node-sass 0.8.3 which is based off of 1122ead)

@media screen and (min-width: 300px) {
  %foo {
    max-width: 80%; } }

As you can see the placeholder %foo should be body but it is not. This is also the same case with multiple selectors extending off of %foo, it still displays %foo instead of the multiple classes that were extended.

I'm quite sure this is only media-query based, I have a reduced test-case with mixins:

@mixin test() {
    @content;
}
%foo {
    @include test() {
        max-width: 80%;
    }
}
body {
    @extend %foo;
}

The output of this is identical in SASS (3.3.1 and 3.2.15) and 1122ead:

body {
  max-width: 80%; }

Further more, multiple mixins also work the same - for example:

@mixin test2() {
    @content
}
@mixin test() {
    @include test2() {
        max-width: 80%;
    }
}
%foo {
    @include test();
}
body {
    @extend %foo;
}

Produces the same output.

[cgarstin]

You can work around this by putting the rules that are inside the mediaquery inside another selector.
E.g change:

%foo {
    @media screen and (min-width: 300px) {
        max-width: 80%;
    }
}

to:

%foo {
    @media screen and (min-width: 300px) {
        &:not(fakeSelector) {
            max-width: 80%;
        }
    }
}

[mattborn]

@cgarstin Why have I never seen this?! While this feels like a hack, it does work.

[digitaljhelms]

Agreed, feels dirty, but awesome workaround for the moment until this is fixed properly. 👍

[mgreter]

Be aware that nested media rules are a CSS3 feature and not supported by CSS2.

[andres-torres-marroquin]

+1 on this issue

[lilymartinez]

+1

[webinsation]

Hey guys there is a better cleaner way. Just use this:

%foo {
    @media screen and (min-width: 300px) {
        & {
            max-width: 80%;
        }
    }
}

Hope this helps all you libsassers trying to get it working!

[mgreter]

This seems to work with latest master! //CC @xzyfer

[xzyfer]

Kind of not really @mgreter . The latest master produce the expected output

@media screen and (min-width: 300px) {
  body {
    max-width: 80%; } }

But not the correct output which is an error message.

[xzyfer]

Never mind my previous comment I was confusing this issue with #317

[xzyfer]

The spec for this is in https://github.com/sass/sass-spec/tree/master/spec/regressions. Do you know what the folder is about @mgreter ?

[mgreter]

Not really. IMHO I recovered this via perl-libsass when sassc did not yet have the ability to discover passing tests itself. So I just moved that folder from todo!

[xzyfer]

I returned the regression specs to their appropriate folders sass/sass-spec#246.

[xzyfer]

This is fixed and will be 3.2.