less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.02k stars 3.41k forks source link

:extend mixins #1177

Open jonschlinkert opened 11 years ago

jonschlinkert commented 11 years ago

This feature request was proposed as a solution to this issue https://github.com/cloudhead/less.js/issues/1155

This is the synopsis

The awesome thing about mixins is that they speed up development, keep my work surface clean, I only have to write them once, and I know what the compiled result will be. However, a disadvantage of mixins is that they aren't DRY. Especially when you use "utility" mixins a lot, like clearfix. This is where extends could be a much better option. In fact, this is a great, concrete use case for extending mixins. It would basically work like extending nested selectors, accept with mixins. So mixins wouldn't change at all, they would still work the same way. If you have a mixin and don't use it, it won't show in the compiled CSS. If you do use it, it's properties will still show up in the compiled code in each selector where it was used. And if you extend a mixin, the selector (or selectors) extending the mixin will show up in place of the mixin. So if you do this:

.clearfix() {
    // stuff
}
.navbar {
    &:extend(.clearfix());
}
.banner {
    &:extend(.clearfix());
}

and the compiled result will be:

.navbar,
.banner {
   // clearfix stuff
}

So the mixins properties are still inherited by the selectors that extended it, but the mixin (selector) itself doesn't show up in the compiled result.

This would make both extends and mixins far more powerful than they are on their own.

lukeapage commented 11 years ago

I like it, but just to be clear.. this doesn't allow you do something you couldn't do before, it just means that you get..

.navbar,
.banner {
    // clearfix stuff
}

rather than

.navbar {
    // clearfix stuff
}
.banner {
    // clearfix stuff
}

or have I misunderstood?

dmi3y commented 11 years ago

I would love have both, parametric and usual mixins, to have

.navbar,
.banner {
    // clearfix stuff
}
.....

or

.clearfix,
.navbar,
.banner {
    // clearfix stuff
}
....

whenever you need, I know this is the simular way how mixins already works, so the question mostly having flexibility and way for better handling the final CSS output in terms of optimization.

surely

.clearfix,
.navbar,
.banner {
    // clearfix stuff
}

is shorter than

 .clearfix {
    // clearfix stuff
  }
 .navbar{
    // clearfix stuff
 }
.banner {
    // clearfix stuff
}

and for more complicated examples that could be critical, and I know some people switched to SASS just because of this shortage

jonschlinkert commented 11 years ago

Regular mixins couldn't change. They are pretty much a fixture of Less. @agatronic, yeah I see it that way as well. There are challenges to this though, like would we be able to extend parametric mixins? And if so how would that work? Assuming this was supported, let's say you extend a parametric mixin twice but use different variables each time, like this:

.transition(@transition) {
  -webkit-transition: @transition;
     -moz-transition: @transition;
       -o-transition: @transition;
          transition: @transition;
}
.navbar {
    &:extend(.transition(opacity .2s linear));
}
.banner {
    &:extend(.transition(opacity .3s linear));
}

I imagine it might create two copies of the mixin, which is no less code, and no advantage over regular mixins:

.navbar {
  -webkit-transition: opacity .2s linear;
     -moz-transition: opacity .2s linear;
       -o-transition: opacity .2s linear;
          transition: opacity .2s linear;
}
.banner {
  -webkit-transition: opacity .3s linear;
     -moz-transition: opacity .3s linear;
       -o-transition: opacity .3s linear;
          transition: opacity .3s linear;
}

However, you might use this particular mixin a lot, so when you use the mixin again on .dropdown with the same variables as .banner, it might result in this:

.navbar {
  -webkit-transition: opacity .2s linear;
     -moz-transition: opacity .2s linear;
       -o-transition: opacity .2s linear;
          transition: opacity .2s linear;
}
.banner,
.dropdown {
  -webkit-transition: opacity .3s linear;
     -moz-transition: opacity .3s linear;
       -o-transition: opacity .3s linear;
          transition: opacity .3s linear;
}

And this is what makes this interesting to me. it would be good to hear feedback from others too

dmi3y commented 11 years ago

@jonschlinkert yes, that's even better illustration, thank you. as i said both hands on that! but understand though it is adding another level of complicity, and needs to be done/not done thoughtfully

krismeister commented 11 years ago

Extension in the method you're reference can be achieved pretty easily with this pattern:


.transition(@transition) {
    -webkit-transition: @transition;
     -moz-transition: @transition;
       -o-transition: @transition;
          transition: @transition;
}

.quickOpacity1(){
    .transition(opacity .2s linear);
}

.quickOpacity2(){
    .transition(opacity .3s linear);
}

.navbar{
    .quickOpacity1();
}

.banner,
.dropdown{
    .quickOpacity2();
}

The above is injecting semi-extended mixins into the the 2 new declarations.

For the clearfix its also easy to use a similar pattern below, which will not only create the clearfix as a class, but inject those clearfix styles into a new declaration.

.clearfix{
  //clearfix stuff
}
.navbar,
.banner {
    .clearfix;
}
jonschlinkert commented 11 years ago

@krismeister I think there might be confusion on this. The pattern you describe is really just nested mixins, or a mixin inheritance pattern. Extends work in the inverse of that.

krismeister commented 11 years ago

@jonschlinkert Mixin inheritence - a great name. It seemed that both of the examples above where trying to inherit output from the parent - which is a core feature of extension. You probably agree that the output of my example was your expected output.

I understand from rereading your original comment however- you'd like the output to be joined into one CSS rule. Though it was written as 2 originally.

jonschlinkert commented 11 years ago

Actually :extend is pretty awesome. I'm going to go full nerd, but it all comes down to "what gets moved where". Think of a mixin like "beaming the mixin's properties down to the selector that used it". And think of :extend as the inverse, "beaming the selector that used it up to the mixin itself." Not the _selector's properties, just the selector itself. So if you did this:

.some-mixin() {
    padding-top: 100px;
    background: #f7f7f7;
}

// a selector that is extending the mixin
.alert:extend( .some-mixin() ) {
    border: 1px solid #e5e5e5;
}
// another selector extending the mixin
section:extend(.some-mixin()) {
    margin: 20px 0;
}

It would result in this:

// The selectors that extended the mixin are now where the mixin used to be.
.alert,
section {
    padding-top: 100px;
    background: #f7f7f7;
}

// And the properties of the mixin did not get copied down below
// so we saved a line or two of code.
.alert {
    border: 1px solid #e5e5e5;
}
section {
    margin: 20px 0;
}

Hopefully that makes more sense. I'm happy to help anytime.

jonschlinkert commented 11 years ago

@agatronic, @matthewdl, @DesignByOnyx, just a thought, I know this is an unusual approach, but assuming there are no technical challenges to this, and assuming we allow a comma separated list of selectors, maybe extend should feel more like an array. What do you think about changing the syntax to: :extend[N].

I think it's easier on the eyes too when mixins are extended:

section:extend[.some-mixin(), .another-mixin()] {
    margin: 20px 0;
}

I'm fine either way, but something just feels right about the square brackets.

lukeapage commented 11 years ago

@jonschlinkert - [] means attribute in css, not array, where as :extend() was chosen because of its link to pseudo selectors, so I strongly think normal brackets should stay.

jonschlinkert commented 11 years ago

good point, I agree.

DesignByOnyx commented 11 years ago

I agree that the brackets looks and reads nice, but yeah we have to stick with syntax we have already chosen.

dmi3y commented 11 years ago

thumb up for extend(N) syntax, more feels like nature CSS, same reason as @agatronic

matthew-dean commented 11 years ago

Extending mixins is not a bad idea. Not sure about the example syntax, @jonschlinkert. Not that I don't like it; I just actually don't understand what you've written.

I think what you mean is that you want to define classes that "pull in" the same mixin content, but don't actually repeat in the resulting CSS. I wouldn't call that extending mixins. That is, you're not altering the mixin definition (in the same way that the extend definition would do to selectors), you're actually altering (extending) the resulting classes.

So, correct me if I'm wrong, but wouldn't your navbar / banner / dropdown example be supported by:

.navbar {
  .transition(opacity .2s linear);
}
.banner {
  .transition(opacity .3s linear);
}
.dropdown:extend(.banner) { }

That is, extending the classes as you normally would? I'm just trying to wrap my head around your usage pattern, and the use of extend for mixins seems at odds with its use and nomenclature for extending selectors. Can you provide further details on why you would need / want to reference the mixin in an extend call, and not extend a selector that referenced the mixin?

dmi3y commented 11 years ago

That's how I get it

DESIRED FINAL CSS OUTPUT

.sometingShared,
.anotherClass,
.anotherYetClass,
.yetClass {
    // amount of shared code here
}

.anotherClass,
.anotherYetClass {
    // did something dynamic with a
}

.yetClass {
    // did something dynamic with b
}

.anotherClass {
    // native another class code
}

.anotherYetClass {
    // native another yet class code
}

.yetClass {
    // native yet class code
}

THE WAY YOU HAS TO GO WITH CURRENT VERSION OF LESS TO GET DESIRED OUTPUT

.somethingShared,
.anotherClass,
.anotherYetClass,
.yetClass {
    // amount of shared code here
}

.someMixin(@val) {
    // do something dynamic with val
}

.anotherClass,
.anotherYetClass {
    .someMixin(a);
}

.yetClass {
    .someMixin(b);
}

.anotherClass {
    // native another class code
}

.anotherYetClass {
    // native another yet class code
}

.yetClass {
    // native yet class code
}

SUGESSTED LESS SYNTAX

.somethingShared {
    // amount of shared code here
}

.someMixin(@val) {
    // do something dynamic with val
}

.anotherClass:extend(.sometingShared, .someMixin(a)) {
    // native another class code
}

.anotherYetClass:extend(.sometingShared, .someMixin(a)) {
    // native another yet class code
}

.yetClass:extend(.sometingShared, .someMixin(b)) {
    // native yet class code
}

While the extends could be useful only when you have really amount of shared code, in other cases ordinary mixins usage have favor.

And just somewhat, live dummy sample, aimed to just playaround

http://jsbin.com/opekon/1/edit http://jsbin.com/opekon/2/edit http://jsbin.com/opekon/3/edit http://jsbin.com/opekon/4/edit

as said, there could be really performance boost in certain circumstances for people who lazy like me:) to optimize LESS code just because it is affected further flexibility

jonschlinkert commented 11 years ago

Here is a nice blog post about extends that covers several things regarding how SASS handles extends, and it's probably wise for us to learn from how others have done this. http://designshack.net/articles/css/semantic-grid-class-naming-with-placeholder-selectors-in-sass-3-2/

In particular, I believe my proposal for extending mixins would essentially achieve the same end result as SASS "placeholders", but without having to create useless, superfluous placeholder-only classes.

EDIT: yes, here is another post that sums up placeholders nicely. It's exactly what I'm proposing with extending mixins http://maximilianhoffmann.com/article/placeholder-selectors

I personally would prefer this above all other extend-related features. Without question. This is a strong statement, but I think I'm not doing a good job of explaining it if others don't agree. This is quite powerful stuff.

dmi3y commented 11 years ago

I feel that SASS

 %tile {
  width: 200px;
  height: 200px;
  margin-right: 20px;
}

is equal of the parametric mixin, designed for extends exclusively, what probably not such a bad in terms of separation of concerns. but yeah that's CSS, and is not it suppose to be as simple as possible?

dmi3y commented 11 years ago

@agatronic while we are talking about, what is this extend https://github.com/agatronic/less.js/blob/master/lib/less/tree/extend.js supposed to be? noted it is already in main less branch. sorry for noob question though.

lukeapage commented 11 years ago

@dmi3y it is the first version of extend which was pulled from a pull request ready for 1.4.0 - it represents the extend node

dmi3y commented 11 years ago

thank you @agatronic, now I see how much I missed ;)

jonschlinkert commented 11 years ago

@dmi3y please feel free to continue adding to the conversation, let's try to keep this issue at a higher level so that we can draw it to a conclusion. Also feel free to email me personally, I'm happy to discuss how mixins and extends work with you (my contact info is on my profile). Also a quick comment to avoid further confusion on this thread. Your example:

%tile {
    width: 200px;
    height: 200px;
    margin-right: 20px;
}

This is the syntax for a SCSS "placeholder", which was implemented in the SCSS spec to accomplish something similar to what I am proposing with extending mixins. But this has nothing to do with being parametric. Parametric mixins are simply mixins that accept parameters, meaning that their values can change each time they are used.

@matthewdl

I think what you mean is that you want to define classes that "pull in" the same mixin content, but don't actually repeat in the resulting CSS

and

wouldn't your navbar / banner / dropdown example be supported by...

No, the operative point is being missed, it's quite the other way around. Maybe the confusion was caused because I made the mistake of focusing on parametric mixins in my example.

So first, I think it's important to delineate that not all mixins are parametric, many are created to not accept parameters. Some mixins, like clearfix, are used over and over again without changing any variables when they are used. These are a perfect use case for why extending mixins would be advantageous.

Parametric or not, in all their glory mixins have a downside in that each time one is used, its properties get duplicated. So let me try to explain how extending mixins would work using only "regular mixins", and continuing with the example you used above.

Given your example, there are two completely independent and unrelated points that need to be made:

  1. the banner class would still show up in the compiled CSS whether is was extended or not, but if the banner class was a mixin it would only show up in the resulting CSS if was used (either extended or mixed in). This alone is valuable. And
  2. When you extend a mixin, rather than duplicating the same exact properties over and over again, we would copy the actual selectors themselves to the place of the mixin.

But this is just one example. Consider that at one point in Twitter Bootstrap the .clearfix() mixin alone was used more than 20 times inside other selectors, AND it was also nested inside 4 or 5 other structural mixins, such as .container-fixed(), .make-row(), and #grid > .core(). Meaning that those mixins were also duplicating the clearfix mixin's properties each time they were used. And this isn't unique to that framework.

Does this make more sense? So in instead of having the properties of the clearfix mixin duplicated more than 20 times, we would group selectors that use the clearfix mixin at the place of the mixin itself, and the properties would only be declared once. And again, this is just one mixin, imagine the net benefit from extending lost of regular mixins, versus having their properties duplicated. As I mentioned somewhere above, regular mixins will still need to be used without being extended sometimes, for specificity and overrides, but that doesn't diminish the value of extending the ones that don't. In fact, I would personally create additional mixins to be used specifically with extend, that would I wouldn't use otherwise.

And my second example above focused on how extend would work with parametric mixins, so hopefully this completes the picture and makes more sense now? If not, the articles I linked to above should clear up any confusion. Extending mixins is extremely powerful and useful and would allow you to achieve the same net result as extending nested selectors, but with mixins specifically instead of any nested selectors. This allows you to organize your selectors in a way that is most likely to provide the results you want, and avoid unintended consequences. @DesignByOnyx, @agatronic, @matthewdl, is there anything I'm missing or forgetting here? Anything we can do to clear up the confusion?

matthew-dean commented 11 years ago

Thanks. I think I understand the use case, and I think this line says it best:

the banner class was a mixin it would only show up in the resulting CSS if was used (either extended or mixed in)

That makes sense to me. Probably what I'm getting hung up is that the overall :extend syntax isn't final yet. But, you make persuasive cases.

I'd say that once we finalize for selector blocks, if the behavior of extending mixins ends up matching the behavior of extending selectors (with the key difference being that line that you just said, that no selector is output in the CSS for an extended UNLESS there is an initial usage), then that's intuitive to me.

That is, if I could also do something like this(or equivalent syntax):

.facebook-button:extend( .button() all ) {
    border: 1px solid #e5e5e5;
}

... then thumbs up from me. But, for me, all of that depends on what/how/if we resolve :extend for selectors. But, syntax aside, as a feature idea, you're right, it's sound.

dmi3y commented 11 years ago

@jonschlinkert thank you, do much appreciate your endorsement, and surely will try my best with putting my thoughts as clearly as I can. I think it is pretty much exiting to hear what's going on with future development and yes, to be heard. You guys all, do awesome work with that.

Yes, this example, I had to be explicit with my previous post.

%tile {
   width: 200px;
   height: 200px;
   margin-right: 20px;
 }

is taken from SASS tutorials provided by Jon, and the main goal of it to avoid trashing output in CSS from classes that would not be in use. So in this particular angle it could be seen like empty parametric mixin in LESS. When you want use it but do not want it was output into end stylesheet.

DesignByOnyx commented 11 years ago

I am jumping in on this discussion and would like to bring clarity for myself on the following comment:

if the banner class was a mixin it would only show up in the resulting CSS if was used (either extended or mixed in)

So, going with a more generic clearfix example, is this the LESS and resulting CSS?:

.clearfix() {
    &:before,
    &:after { content: " "; display: table; }

    &:after { clear: both; }
}
.something:extend( .clearfix() ) { }

/*
.clearfix:before,
.clearfix:after,
.something:before,
.something:after {
    content: " ";
    display: table;
}
.clearfix:after,
.something:after {
    clear: both;
}
*/

I originally expected for the .clearfix class to remain omitted from the compiled CSS. Personally, I would not need the class in the compiled CSS, as I would not be using it in markup and it's extra bytes in my CSS. I don't really have a strong opinion either way, but clarification on this point would help me make better comments moving forward. Thanks.

jonschlinkert commented 11 years ago

is this the ... resulting CSS?:

I'm glad you're asking, but no, only the .something class would show up in the resulting CSS. So you were correct in your original expectation.

lukeapage commented 11 years ago

just read the placeholders link and yes I think extending mixins is a better and more less way to do things and get the same functionality.

To calrify your above point

.clearfix() {
    &:before,
    &:after { content: " "; display: table; }
    &:after { clear: both; }
}
.something:extend( .clearfix() ) { }

/*
.something:before,
.something:after {
    content: " ";
    display: table;
}
.something:after {
    clear: both;
}
*/

and presumably if .clearfix() { definition is .clearfix { then calling it with or without brackets inside the extend definition will make no difference to the output..

hax commented 11 years ago

Using pseudo class syntax for extend feature is totally WRONG, because "extend sth" is NOT equal to "match sth", it means nothing about DOM structure which a selector should.

You LESS guys have "invented" many bad ideas. The most famous one is "reuse' .xxx (class selector) as mixin notation. At least it ease the migration from plain CSS to preprocessor. But :extend syntax is worst idea I ever heard.

jonschlinkert commented 11 years ago

I don't want to dignify this kind of rant on our Issues, but this isn't the first time I've heard these things so let's talk about CSS and clear the air.

You LESS guys have "invented" many bad ideas. The most famous one is "reuse' .xxx (class selector) as mixin notation. .... :extend syntax is worst idea I ever heard.

.xxx is called an implicit mixin. It works, and it's easy to understand. In fact, this syntax is not as "invasive" on the CSS spec as, say, using at-rules for styling actual. @keyframes come the closest to what would be described as styling and could maybe be considered an exception to what I'm saying since identifiers are used which match the identifier production in CSS syntax. Beyond keyframes, in general CSS at-rules are reserved mostly for higher-level configuration and style- or stylesheet manipulation based on "external" information, meaning outside of the styles themselves, like character encoding (@charset), or responding to device-specific conditions (@media).

Besides, I can think of worse ideas. Like using at-rules for styling is one of them, using two at-rules for the same feature is another. Of course, it all depends on your goals, if you want to programmatically manipulate styles without touching them personally, then there are probably advantages to using more obtuse and obvious grammar constructs that might be easier for a parser to pick up. Looks like you use Stylus, so I'm interested in hearing your point of view on why other syntaxes are advantageous.

"extend sth" is NOT equal to "match sth", it means nothing about DOM structure which a selector should.

Correct. Pseudo-classes with nth something are described as "structural" pseudo-classes by the CSS specification. However, there are 6 other types of pseudo-classes: "dynamic", "negation", "target", "blank", "UI element states" and "language". If the CSS specification did in fact describe an "extend" feature, it could sit quite elegantly as a category between the "negation" and "target" pseudo-classes.

Pseudo-classes allow selection based on information in the document tree, but this is probably not optimal use of the extend feature.

totally WRONG

A community that is larger than all the other preprocessors combined can't be wrong. What else is there to say?

DesignByOnyx commented 11 years ago

@hax - Mindless trolling could turn into an educated discussion if you proposed your own solution like everybody else on the :extend threads. After weeks of different suggestions and deliberation between very experienced front-end devs and heavy users of LESS, we came to a satisfactory solution with the :extend syntax. If you have a better suggestion, believe me - we are all ears.

matthew-dean commented 11 years ago

To add to @jonschlinkert, .this:not(.that) { } and .this:matches(.that) { } and .this:extend(.that) { } are categorically similar. That is: "Given selector 'this', relate it to selector 'that' in a particular way." Since LESS often extends concepts and syntax introduced in CSS, it's a pretty logical extension of the language. We pay close attention to the CSS spec, and as a result, mirror its idiosyncrasies. If there's oddness in the concept, it hopefully starts in CSS first. That's why in our mixin guards, "AND" is represented by the word "and", and "OR" is represented by a comma: because that's the way it is in media queries. Our goal is not to fix or change CSS, but make CSS far easier to work with, and to make LESS completely familiar for CSS authors. :extend is an example of that. If you know how to use :not, you know how to use :extend.

jonschlinkert commented 11 years ago

What if we used a more "implicit" syntax for this to avoid nesting parens and to make it easier to extend parametric mixins?

Today, we use a parametric mixin like this:

.square {
  .border-radius(9px);
}

Rather than extending the mixin like this (as you might extend a normal class):

.square {
  &:extend(.border-radius);
}

We could extend like this:

.box {
  .border-radius:extend(9px);
}
.square {
  .border-radius:extend(9px);
}
.rectangle {
  .border-radius:extend(4px);
}

The distinction is that the "extended mixin" ends in a semicolon and declares a value or values inside the parens, .border-radius:extend(9px);, rather than listing the classes to extend inside the parens and beginning a new selector block with curly braces, .border-radius:extend(.some-class) {}. IMHO, this is no less subtle than implicit mixins (.border-radius;), which is one of LESS's coolest features, again IMHO.

In the rendered output, any "extended parametric mixin" would be rendered at the place of the original mixin, like this:

.box, 
.square {
    -webkit-border-radius: 9px;
    -moz-border-radius: 9px;
    border-radius: 9px;
}
.rectangle {
    -webkit-border-radius: 4px;
    -moz-border-radius: 4px;
    border-radius: 4px;
}
.some-other-class, 
.another-class, 
.and-another {
    -webkit-border-radius: 2px;
    -moz-border-radius: 2px;
    border-radius: 2px;
}

So I personally like this because it's enough of a departure from the "normal" extend syntax inside selector blocks to make it obvious enough what's happening, and it goes along with the subtlety of existing less conventions where there is no need to over-explain what you're trying to accomplish in the code. This feature is definitely at the top of my personal wishlist.

jonschlinkert commented 11 years ago

Using the clearfix example I gave in my original request, this is what it would look like with my revised syntax:

.clearfix() {
    // ...
}
.navbar {
    .clearfix:extend(); // instead of &:extend(.clearfix());
}
.banner {
    .clearfix:extend();
    &:extend(.some-class); // "implicit mixin" being extended
}
DesignByOnyx commented 11 years ago

I like where you are going with this. I think a more friendly approach is to use an :extend on the end of existing mixin syntax as such this:

.box {
  .border-radius(9px):extend;
}
.square {
  .border-radius(9px):extend;
}
.rectangle {
  .border-radius(4px):extend;
}

Something about omitting the parens after the :extend makes if feel like it serves a different purpose than the existing extend syntax.

jonschlinkert commented 11 years ago

I could go any way with this, but I prefer .border-radius:extend(9px); because it's consistent with extend syntax, IMO it's still obvious that it's a mixin, but it also implies that the parameters/values are being extended - which will help explain why the output is "grouped".

DesignByOnyx commented 11 years ago

@jonschlinkert - by this point I hold your opinions in very high regard and will support any decision you think is best. I will say that your method feels as though the 9px is being passed to "extend" as opposed to being passed to the mixin itself... but I am cool with either.

jonschlinkert commented 11 years ago

@DesignByOnyx thanks for the kind words. What you're saying makes a lot of sense, my "lens" of extends (in Less) has been something like:

.this:extend(.that) {}

so you're point is consistent with my own view. @matthewdl or @lukeapage do either of you have any view/opinion on this? I think both syntaxes could work (and probably others), but admittedly I haven't focused on "future-proofing" with either syntax, so there could be some gotchas on either. I'd just love to see this happen, I think extending mixins is an exciting feature for Less, it will do more for generating DRY code than any other feature I can think of at the moment

bdkjones commented 11 years ago

Why can't the Less compiler just automatically "do the right thing"? In other words, if I have this mixin declared in my code:

.someMixin {
    color: red;
}

And then later I use it like this:

#someElement {
    .someMixin;
}

Why can't the compiler just be smart enough to optimize this Less code into this CSS:

.someMixin,
#someElement
{
    color:red;
}

Why should I, as the user, have to manually add an :extends keyword to get this behavior? The compiler should be smart enough to realize that this optimization can be done and then do it.

Personally, I think this :extends idea is confusing. The whole notion of an "extends" keyword means: "Take this thing X and add A, B, and C to it." But in this context, you propose to re-define extends to: "This thing X is actually the same as Y, so you can just declare Y and X to be equal and write it once." This goes against the standard notion of "extending" something.

jonschlinkert commented 11 years ago

Yeah, I was thinking that same exact thing earlier and was going to post it, but after thinking about it decided that automatically applying "extend behavior" to mixins is too opinionated for Less.js. Specifically, even though selector inheritance (extend) is an awesome concept that is super beneficial for code reduction, the vast majority of the complaints I've read about the feature is that it adds a layer of abstraction that makes it difficult to comb through code and debug when there are problems.

So great minds think alike, but I down-vote the idea personally because a) we shouldn't mess with "normal" mixins at all until the extend feature is completely stable and the spec is more complete, b) I think many developers want to make this call for themselves, and c) oftentimes things seem so obvious and cut-and-dried, when the fact of the matter is that we're forgetting about many of the eccentricities about mixins that would make your suggestion difficult to implement at minimum, and probably impossible to implement whilst still enabling developers to use all of the "mixin hacks" we've... ahem, they've come to know and love.

bdkjones commented 11 years ago

The :extends() syntax is still a pretty clumsy hack that's unintuitive and difficult to explain to new users.

The better solution is to simply make the compiler automatically optimize the CSS as described above when the user has selected an appropriate output style (compressed, optimized, etc). If the user selects the uncompressed output style, then the compiler should avoid these optimizations and generate less DRY code.

Plus, you can solve the debugging difficulties with source maps. They can easily point out the location of the compiled CSS that comes from a Less mixin.

jonschlinkert commented 11 years ago

@bdkjones please create a new feature request for "automatically optimizing the CSS" with your suggestions for how to accomplish this in the code. This kind of input is always welcome.

bdkjones commented 11 years ago

Point taken! I didn't mean to be quite so harsh in my last comment; I get carried away sometimes.

As for code, I'm afraid I'm totally busy with CodeKit. I let you guys hash out the languages!

jonschlinkert commented 11 years ago

Not at all! really, I saw you're involvement with CodeKit, would love to have your continuing input. Not all features are going to please everyone, but feedback and strong opinions are the only things that keep the wheels turning!

matthew-dean commented 11 years ago

@bdkjones The problem is the cascade. In your use case, it works, but selector grouping wouldn't have the desired result in all cases, since it would possibly move selectors "earlier" in the document preventing declarations that were overwriting those selectors.

But, I agree with your statements against the mixin extend syntax as it has evolved here. Selector X should extend Selector or mixin Y, so this seems weird:

.border-radius(9px):extend;

I think we're misusing the extend syntax. If the extend syntax as it exists doesn't fit with mixins (which is perfectly plausible), I'd suggest we find the right syntax for mixins, rather than expand the extend syntax in a way that mangles the metaphor, as @bdkjones points out.

jonschlinkert commented 11 years ago

I think we're misusing the extend syntax

I disagree. If it works, use it. Why create new syntaxes for no reason? I think we have great precedent for the proposed syntax, I give copious examples and supporting reasons behind my suggestions, but your opinion about the proposed syntaxes seems very subjective. And since you've made that statement a couple of times recently, can you explain why you think the syntax is being abused? Or explain at what point does a syntax being being abused? Or share your thoughts on how mixins should be extended, but with out the "extend" syntax?


In your use case, it works, but selector grouping wouldn't have the desired result in all cases, since it would possibly move selectors "earlier" in the document preventing declarations that were overwriting those selectors. ... I think we're misusing the extend syntax. If the extend syntax as it exists doesn't fit with mixins (which is perfectly plausible), I'd suggest we find the right syntax for mixins, rather than expand the extend syntax in a way that mangles the metaphor, as bdkjones points out.

No. He didn't point anything out, he urged us to make Less easier to use - that was his bottom line. His suggested way of doing so was misguided, but the intent was good. What he wasn't thinking about is the cascade that you just pointed out. When using extend in general you must take the cascade into account. I think anyone who uses extend instead of a mixin is aware of that. Thus, that's why Less.js offers you the choice of either using it when it makes sense, or not when it doesn't. The fact that the first "C" in CSS stands for cascading doesn't negate the advantage of consolidating duplicate styles when the cascade isn't material. What am I missing here?

So can you clarify your sentiment? Are you against this feature, or the syntax? If you're against the syntax then please propose something else to keep things moving, otherwise let's not hinder progress of a feature that is this promising for the language.

matthew-dean commented 11 years ago

Are you against this feature, or the syntax?

I am for the feature, absolutely. This is an obvious direction. I reviewed the syntax again in this thread. Some thoughts:

First, this is awkward to me:

.something:extend( .clearfix() ) { }

The double parentheses are just weird. But, it is a faithful adoption of the existing extend syntax, so fair enough. So, that's why I was advocating for something else, perhaps something other than extend.

Then there's this which you proposed, I think for similar reasons:

.border-radius(9px):extend;

...which I had an allergic reaction to, because it seems contrary to the existing extend spec. So that's where it seemed like misuse to me.

BUT

I realized there's actually two options there. One is that we do something different for mixins. But the other option is that we revise the existing extend spec, for all things, including mixins, such as:

#namespace {
  .box-template {
    .subelement {
      // we'll add the all flag to extend everything here
    }
  }
}
.clearfix() {
  // a clearfix mixin
}
.text-shadow(@shadow) {
  -moz-text-shadow: @shadow;
  text-shadow: @shadow;
}
.some .random .selector {
  // with properties
}
.mybox {
  .clearfix:extend;
  #namespace > .box-template:extend !all;
  .text-shadow:extend(2px 2px #ff0000);
  .some .random .selector:extend;
}

But.... with a full selector (.some .random .selector), it starts to read wrong. It looks like just .selector has the extend on it. And the word "extend" starts to get lost, when it's the important word in this declaration. This wasn't the case when it was written as &:extend() (because it appeared at the beginning), but that syntax seems to break down with mixins.

So, I would propose one of these options:

// Migrating existing syntax, but ommitting a parens requirement when used with &:

.mybox {
  &:extend .clearfix;
  &:extend #namespace > .box-template !all;
  &:extend .text-shadow(2px 2px #ff0000);
  &:extend .some .random .selector;
}

// Adopting something similar to the SASS approach

.mybox {
  @extend .clearfix;
  @extend #namespace > .box-template !all;
  @extend .text-shadow(2px 2px #ff0000);
  @extend .some .random .selector;
}

I think the discussion around extending mixins and media queries has highlighted that our original syntax approach to :extend is not that flexible, especially when we want to extend multiple selectors & mixins.

I think it's important to get :extend right. If we want to use "extend" for mixins, then I think we have to revise extend syntax.

jonschlinkert commented 11 years ago

I didn't propose .border-radius(9px):extend;, @DesignByOnyx did. I proposed .border-radius:extend(9px);

But either syntax is fine with me. Or we could do .border-radius(9px) !extend; or .border-radius:shamallamadingdongscoobiedoo(9px):extendmyass (jk), it doesn't matter to me because nothing changes with the behavior of mixins or extend at all.

So I'm not sure how you're coming up with the need to do all that, you're probably over-complicating things for yourself.

This feature request is expressly about having the ability to copy the inherited properties of a mixin to the place of the mixin, and it will follow rules that have already been well established with current mixin behavior, and now also have precedence with and identical behavior to the @import (reference) feature (which, btw, I've been using quite a bit in practice and love). In a nutshell:

I'm not sure why you even want to rehash the debate on the syntax for extend after two years of debate, > 100 posts, a lot of research and the case has already been made that it's much more flexible than @extend.

Btw, I'm not sure what you mean here:

with a full selector (.some .random .selector), it starts to read wrong. It looks like just .selector has the extend on it. And the word "extend" starts to get lost, when it's the important word in this declaration.

I don't agree that it's confusing, since that's how it actually works now. Isn't that like saying "CSS developers don't know how grouped selectors work"? What about :hover or :after? I think CSS devs can figure this out fine, because it works the same way as other pseduos.

.some .random .selector:extend(.something) {}

I like the flexibility this affords, since I can either (or both) apply extend to the selector, which is .some .random .selector, not .selector, or I can tuck it inside the selector block.

I think the discussion around extending mixins and media queries

I closed the media queries issue because I realized it could be solved with extending mixins. "All roads lead to _____" extending mixins. ;-)

DesignByOnyx commented 11 years ago

I would like throw in my two cents say that the parens inside a parens - :extend( .some-mixin() ) - doesn't really bother me that much, especially since :extend( .some-selector ) is already established and has been discussed ad nauseam. I agree that the double parens not ideal by any means and it does feel kinda weird. But I feel that it's important that mixins and selectors be treated the same in terms of "extending", much the same way they are treated the same in classic Less:

header {
    .some-selector;
    .some-mixin;
    .some-mixin();
}

As such, I don't really feel too bad (or limited) by doing this:

header {
    &:extend( .some-selector );
    &:extend( .some-mixin );
    &:extend( .some-mixin() );
}
matthew-dean commented 11 years ago

But I feel that it's important that mixins and selectors be treated the same in terms of "extending", much the same way they are treated the same in classic Less

I'm on the same page on that point.

I'm not sure why you even want to rehash the debate on the syntax for extend after two years of debate, > 100 posts

It's not a matter of wanting to. No, I don't want to. Yet at the time, we sort of shuffled off mixins to be dealt with later. I just want to ask the question if we're sure this syntax works in the way we want it to work for everything. There's nothing wrong with revisiting the question if we feel that it doesn't work or it's inelegant for some uses. LESS has existed a long time without extend. It's more important to get it right than to get it done quickly.

So, we're cool with this?

.mybox {
  &:extend(.clearfix());
  &:extend(#namespace > .box-template all);
  &:extend(.text-shadow(2px 2px #ff0000));
  &:extend(.some .random .selector);
}
jonschlinkert commented 11 years ago

parens inside a parens - :extend( .some-mixin() ) - doesn't really bother me that much, especially since :extend( .some-selector ) is already established

Yes true, and point in fact we already have precedent for double parens in Less.js as well, AND with existing extend syntax. And the precedent that has already been set is not as easy on the eyes as what we're suggesting with mixins. Currently, we can do this:

.one:nth-child(5) {
  color: red;
}
// We can extend psuedos
.two:extend(.one:nth-child(5)) {
  background: blue;
}
// And attributes
.two:extend(.one, [hidden]) {
  width: 2px;
}

You will even find nested parens in existing CSS syntax.

So here we have both nested parens and nested pseudo-class syntax. And I have no problem with it, it's clear IMO.

So I agree with @DesignByOnyx, that was my original proposal and it seemed distasteful to others for some reason, so I was trying to find another syntax. But I'm all for just using regular old mixin syntax right inside the extend's parens.

In fact, the original syntax would would allow multiple comma separated classes or mixins to be extended.

.banner {
    &:extend(.clearfix(), .border-radius(4px), .alert, .navbar-fixed-top, [hidden]); 
}

It does the job, and it's tight. No complaints here.

jonschlinkert commented 11 years ago

So, we're cool with this?

yep. completely. That's already the way it is in CSS. So I'm good with it.