Disallow @extend within directives #154

Closed
nex3 opened this Issue Aug 10, 2011 · 16 comments

Comments

Projects
None yet
7 participants
@nex3
Contributor

nex3 commented Aug 10, 2011

As per discussion in #96, @extend behaves in unintuitive ways when used within directives such as @media. Since there's no acceptable way to resolve this behavior short of browser-supported @extend, we'll just disallow it.

The plan is to add a deprecation warning in 3.1.x and change that to a full-on error in 3.2.

@ZeeAgency

This comment has been minimized.

Show comment
Hide comment
@ZeeAgency

ZeeAgency Sep 14, 2011

So... no plans for non-global @extend ?

So... no plans for non-global @extend ?

@nex3

This comment has been minimized.

Show comment
Hide comment
@nex3

nex3 Sep 14, 2011

Contributor

@ZeeAgency That's pretty tangential to this issue, but no, currently we don't have any concrete plans for that. We're tossing around ideas for a more advanced @import mechanism that may allow for file-scoped @extend in the future, but the details remain to be seen.

Contributor

nex3 commented Sep 14, 2011

@ZeeAgency That's pretty tangential to this issue, but no, currently we don't have any concrete plans for that. We're tossing around ideas for a more advanced @import mechanism that may allow for file-scoped @extend in the future, but the details remain to be seen.

@gxclarke

This comment has been minimized.

Show comment
Hide comment
@gxclarke

gxclarke Sep 15, 2011

As an alternative to disallowing @extend within @media, do you see any issues with duplicating the selector when used across a different media query scope and then only merge those that are scoped within the same media query? For example:

.test { a: b; }
@media only screen and (max-width: 500px) {
   .media-test { @extend .test; c: d;}
   .media-test-2 { @extend .test; e: f; }
}

Generates:

.test { a: b; }
@media only screen and (max-width: 500px) {
    .test, .media-test, .media-test-2 { a: b; }
    .media-test { c: d; }
    .media-test-2 { e: f; }
}

Off-hand I can't think of scenarios in which this would behave in an unintuitive way. The processing could be smartened up further to exclude the additional reappearance of .test when the base class is defined outside a media query:

.test { a: b; }
@media only screen and (max-width: 500px) {
    .media-test, .media-test-2 { a: b; }
    .media-test { c: d; }
    .media-test-2 { e: f; }
}

As an alternative to disallowing @extend within @media, do you see any issues with duplicating the selector when used across a different media query scope and then only merge those that are scoped within the same media query? For example:

.test { a: b; }
@media only screen and (max-width: 500px) {
   .media-test { @extend .test; c: d;}
   .media-test-2 { @extend .test; e: f; }
}

Generates:

.test { a: b; }
@media only screen and (max-width: 500px) {
    .test, .media-test, .media-test-2 { a: b; }
    .media-test { c: d; }
    .media-test-2 { e: f; }
}

Off-hand I can't think of scenarios in which this would behave in an unintuitive way. The processing could be smartened up further to exclude the additional reappearance of .test when the base class is defined outside a media query:

.test { a: b; }
@media only screen and (max-width: 500px) {
    .media-test, .media-test-2 { a: b; }
    .media-test { c: d; }
    .media-test-2 { e: f; }
}
@chriseppstein

This comment has been minimized.

Show comment
Hide comment
@chriseppstein

chriseppstein Sep 16, 2011

Member

@gxclarke this is an interesting idea and theoretically sound, but it's more than just copying the base class, it's all the selectors that involve the .test class and if those are in media queries... well it gets complicated fast.

Member

chriseppstein commented Sep 16, 2011

@gxclarke this is an interesting idea and theoretically sound, but it's more than just copying the base class, it's all the selectors that involve the .test class and if those are in media queries... well it gets complicated fast.

@scottkellum

This comment has been minimized.

Show comment
Hide comment
@scottkellum

scottkellum Apr 19, 2012

Any way to let a mixin or function know if it is within a directive so styles can be written correctly instead of extending a selector outside of the directive?

=width($n)
  @if directive
    +width($n)
  @else
    @extend %width-#{$n}

Seems weird though. Bummer, I really wanted this to work and got way too excited for extending within media queries.

Any way to let a mixin or function know if it is within a directive so styles can be written correctly instead of extending a selector outside of the directive?

=width($n)
  @if directive
    +width($n)
  @else
    @extend %width-#{$n}

Seems weird though. Bummer, I really wanted this to work and got way too excited for extending within media queries.

@chriseppstein

This comment has been minimized.

Show comment
Hide comment
@chriseppstein

chriseppstein Apr 19, 2012

Member

@scottkellum we need to make the case for getting @extend in browsers.

Member

chriseppstein commented Apr 19, 2012

@scottkellum we need to make the case for getting @extend in browsers.

@scottkellum

This comment has been minimized.

Show comment
Hide comment
@scottkellum

scottkellum Apr 19, 2012

@chriseppstein This stuff seriously changed the way I write code (wrote an unpublished article and framework about this). File size seems better when preprocessing @extend although native mixins would do wonders for CSS size.

@chriseppstein This stuff seriously changed the way I write code (wrote an unpublished article and framework about this). File size seems better when preprocessing @extend although native mixins would do wonders for CSS size.

@scottkellum

This comment has been minimized.

Show comment
Hide comment
@scottkellum

scottkellum Apr 20, 2012

To push just a little harder to why I think this is such an important, here is an excerpt from an article I am working on explaining how @extend is saving me tons of K on file size when used correctly. Basically nothing is written unless it is used and @extend on @media is the crux of my responsive mobile first workflow with no fixed breakpoints:
https://gist.github.com/2430955

When used improperly @extend can add a lot of code, but it is helping me keep my file size way down.

Because the actual base CSS objects are never written to save space, there would be nothing to extend inside media queries making this implementation impossible outside of preprocessors. From this mobile first approach almost every use of @extend will exist inside of a media query and the original object never written to the CSS.

In practice incredibly complex operations can be done that write very little code: https://github.com/scottkellum/Singularity

OK, done pushing, would love to help figure out the logic if that is whats needed :)

To push just a little harder to why I think this is such an important, here is an excerpt from an article I am working on explaining how @extend is saving me tons of K on file size when used correctly. Basically nothing is written unless it is used and @extend on @media is the crux of my responsive mobile first workflow with no fixed breakpoints:
https://gist.github.com/2430955

When used improperly @extend can add a lot of code, but it is helping me keep my file size way down.

Because the actual base CSS objects are never written to save space, there would be nothing to extend inside media queries making this implementation impossible outside of preprocessors. From this mobile first approach almost every use of @extend will exist inside of a media query and the original object never written to the CSS.

In practice incredibly complex operations can be done that write very little code: https://github.com/scottkellum/Singularity

OK, done pushing, would love to help figure out the logic if that is whats needed :)

@nex3

This comment has been minimized.

Show comment
Hide comment
@nex3

nex3 Apr 20, 2012

Contributor

@scottkellum It's not a logic issue, it's a bloat issue. It's dangerous to have a construct that produces a relatively manageable amount of output in most contexts but hugely more in certain specific contexts. To say nothing of the potential behavioral issues of re-ordering the document.

For your case in particular, it really feels as though all the %width-* rules should be mixins. Conceptually, they aren't classes to be inherited, they're just presentational rules. Is there a reason that they're placeholder selectors? Is it just for code size?

Contributor

nex3 commented Apr 20, 2012

@scottkellum It's not a logic issue, it's a bloat issue. It's dangerous to have a construct that produces a relatively manageable amount of output in most contexts but hugely more in certain specific contexts. To say nothing of the potential behavioral issues of re-ordering the document.

For your case in particular, it really feels as though all the %width-* rules should be mixins. Conceptually, they aren't classes to be inherited, they're just presentational rules. Is there a reason that they're placeholder selectors? Is it just for code size?

@scottkellum

This comment has been minimized.

Show comment
Hide comment
@scottkellum

scottkellum Apr 21, 2012

@nex3 yes it is just for code size. I guess gzip would solve that issue but I would love to handle it on the CSS level.

@nex3 yes it is just for code size. I guess gzip would solve that issue but I would love to handle it on the CSS level.

@aaronjensen

This comment has been minimized.

Show comment
Hide comment
@aaronjensen

aaronjensen May 17, 2012

@scottkellum There's another way to do this that works even with this change and I would argue makes the code better organized.

%width-1 { ... }

article { @extend %width-1, %big-width-2 }

@media all and (min-width: 20em) {
  %big-width-2 { ...}
}

In that way your article selector contains all of its extensions in one place.

@scottkellum There's another way to do this that works even with this change and I would argue makes the code better organized.

%width-1 { ... }

article { @extend %width-1, %big-width-2 }

@media all and (min-width: 20em) {
  %big-width-2 { ...}
}

In that way your article selector contains all of its extensions in one place.

@Anahkiasen

This comment has been minimized.

Show comment
Hide comment
@Anahkiasen

Anahkiasen May 17, 2012

I also think you should be able to use placeholders and @extend in media queries as long as the placeholders have been defined in said media query. When you think about it, it makes sense : media queries create a sort of frame — a different environnement, while in the same CSS.

Sass should just aknowledge that and treat the media queries as a separate environnement, and while still able to use @extend, if you'd went and try to extend a placeholder/selector that is outside the media query, it would just throw an error.

I also think you should be able to use placeholders and @extend in media queries as long as the placeholders have been defined in said media query. When you think about it, it makes sense : media queries create a sort of frame — a different environnement, while in the same CSS.

Sass should just aknowledge that and treat the media queries as a separate environnement, and while still able to use @extend, if you'd went and try to extend a placeholder/selector that is outside the media query, it would just throw an error.

@nex3

This comment has been minimized.

Show comment
Hide comment
@nex3

nex3 May 17, 2012

Contributor

@Anahkiasen that's the subject of issue #391

Contributor

nex3 commented May 17, 2012

@Anahkiasen that's the subject of issue #391

@Anahkiasen

This comment has been minimized.

Show comment
Hide comment
@Anahkiasen

Anahkiasen May 18, 2012

My bad, it's been a while since I checked the issues list.

My bad, it's been a while since I checked the issues list.

@scottkellum

This comment has been minimized.

Show comment
Hide comment
@scottkellum

scottkellum May 18, 2012

@aaronjensen unfortunately I would like to reuse the same grid objects throughout the breakpoints fluidly instead of generating a set of objects for each breakpoint.

Also, extending from within the @media directive with the rest of your styles results in cleaner CSS output. These directives add a fair amount of weight to a page and the goal is to write one for each breakpoint. Excessive media bubbling can result in duplicate @media declarations.

@aaronjensen unfortunately I would like to reuse the same grid objects throughout the breakpoints fluidly instead of generating a set of objects for each breakpoint.

Also, extending from within the @media directive with the rest of your styles results in cleaner CSS output. These directives add a fair amount of weight to a page and the goal is to write one for each breakpoint. Excessive media bubbling can result in duplicate @media declarations.

@aaronjensen

This comment has been minimized.

Show comment
Hide comment
@aaronjensen

aaronjensen May 18, 2012

@scottkellum I'm not sure what you mean by "Excessive media bubbling" or "duplicate @media declaration". Could you clarify? My proposal doesn't involve any additional @media declarations, and it keeps all of the style about one rule in one place, so it's easier to understand.

@scottkellum I'm not sure what you mean by "Excessive media bubbling" or "duplicate @media declaration". Could you clarify? My proposal doesn't involve any additional @media declarations, and it keeps all of the style about one rule in one place, so it's easier to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment