less / less.js

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

The way Extend works - more complicated selectors and change in matching #1155

Closed lukeapage closed 11 years ago

lukeapage commented 11 years ago

From @DesignByOnyx

I have finally gotten around to testing this and wanted to post my initial findings. I have been using the alpha version less-1.4.0-alpha.js on the client-side only (no command line yet - not that it should be any different really).

So far, everything works as I would expect except for two major drawbacks which can be explained with one example. Take the following code which is located in one of my root LESS files:

.container { margin: 0 auto; }
 .container.positioned { position: absolute }

@media screen and (min-width: 30em) {
    .container.positioned { left: 50%; }
}
@media screen and (min-width: 30em) and (max-width: 48em) {
    .container { width: 30em; }
    .container.positioned { margin-left: -15em; }
}
@media screen and (min-width: 48em) and (max-width: 60em) {
    .container { width: 48em; }
    .container.positioned { margin-left: -24em; }
}
@media screen and (min-width: 60em) {
     .container { width: 60em; }
     .container.positioned { margin-left: -30em; }
}

Issue 1 - styles defined within media queries do not get extended. Anything trying to extend the .container class only gets the margin: 0 auto styles.

.some-element:extend(.container);

Issue 2 - compound selectors do not get extended. However, the first participant DOES get extended. For example, the following incorrectly extends the .container styles but not the intended .container.positioned styles.

.some-element:extend(.container.positioned);

I wish I could provide a solution. Hope this helps.

DesignByOnyx commented 11 years ago

I have a request for the moderator so that someone doesn't try and use my code in their projects. It turns out that specifying media queries in rem doesn't work in some browsers (Safari, iPad Chrome, iPad Safari, etc). This was an experimental feature I was testing. Could you please change the values in the media query definitions to em. The .container width and margins can still use rem (and should).

matthew-dean commented 11 years ago

@DesignByOnyx Updated.

DesignByOnyx commented 11 years ago

And to complicate matters more, here's another weird case:

.shared-style { ... }

@media screen and (min-width: 48em) {
     .some-element:extend(.shared-style) { ... }
}

The situation here is that .some-element always gets the shared styles as opposed to only when the media query is active:

.shared-style,
.some-element { ... }

@media screen and (min-width: 48em) {
     .some-element { ... }
}

The only real solution I see here is to abandon :extend within media queries and resort to the classic include:

.shared-style { ... }

@media screen and (min-width: 48em) {
     .some-element { 
         .shared-style;
     }
}
lukeapage commented 11 years ago

issue 2 is just a missing feature.. surprised it parses as the contents of :extend( should be a single class or node selector. maybe a feature request for the future.

media queries.

  1. if you extend a class that is in a media query it should "just work" - it clearly doesn't and I agree with you
  2. if you do an extend inside a media query shouldn't it copy the code inside the media query? E.g. in your simple example it would act the same as .shared-style; but in more complex examples it would pull in the bits of the selector too
DesignByOnyx commented 11 years ago

Regarding your point number 1 - I think selector matching isn't a feature but a requirement. Can we not pull parts of sizzle in so that all selector patterns get matched?

Regarding your point 2 about copying the code - I agree, but the problem gets very complicated if you have something like this, as the parser will have to be smart enough to apply styles from other media queries:

-- take note of the single :extend below, then take note of the comments --

/* These styles should be copied */
.shared-style { ... } 

@media screen and (min-width: 30em) {
    /* These styles should be copied */
    .shared-style { ... } 
}
@media screen and (max-width: 30em) {
    .shared-style { ... }
}
@media screen and (min-width: 48em) {
    .some-element:extend(.shared-style) {
        /* Copied styles should be pasted here  */
    }
}
@media screen and (min-width: 60em) {
    .shared-style { ... }
}
@media screen and (max-width: 60em) {
    /* These styles should be copied */
    .shared-style { ... } 
}
jonschlinkert commented 11 years ago

@DesignByOnyx, I'm going to have to spend more time with your use case, but honestly after using SASS extends and doing a lot of testing on extends myself, I'm not sure I agree with any of your point(s) about number 1 (that's the only part I'm responding to here). The "comprehensive selector matching" was something I personally don't like - and didn't like about SASS. Why would you want to extend to every selector pattern? I really am curious, that's not intended as a statement...

From my standpoint, the Less.js implementation and syntax for extends are far more powerful than SASS and allow you to use a surgical "laser" approach with design practices, rather than using than a shotgun blast. Seems like the "sizzle" is very specifically the ability choose what you want to extend.

However, I'm open-minded about it and I sincerely want to know if I'm missing a crucial point about extends. It seems like you might be thinking about ways of using it that hadn't occurred to me before. Can you provide some real-world use cases where this is a good practice? Maybe link to a gist?

In the meantime, here is write up about learning SASS extends that brings some important points to light: http://8gramgorilla.com/mastering-sass-extends-and-placeholders/

Some quotes from the write up "A problem of using a standard class selector as your @extend identifier is that you extend any styles that feature that selector anywhere in your Sass.", and "To me, I think it would be better that when you extend a selector, you only extend that specific selector."

And there are a number of other blogs, discussions and stackoverflow answers that state the same things. And usually the spirit of the opinion on SASS extends can be summed up by another quote from that link: "I’m not convinced I like that @extends work like this but that’s how it is and it’s better to know exactly how it works."

DesignByOnyx commented 11 years ago

I think you are jumping in on a discussion and have missed a little of the context here. We have 3 issues really:

1) LESS is not adding complex selectors (eg .compound.selectors) to the tree... which I consider a bug and should be fixed immediately. The regex is too simple... we should borrow code from the sizzle engine to fix this. 2) The :exend feature does not extend a selector within media queries... which kinda makes the feature a little useless. 3) An :extend within a media query is extending a non-media query selector... which is wrong.

I would like to take this opportunity to say more-specific selectors should not get extended in the way that SASS does. For example, in the example below, the .header .selector should NOT get extended (in my opinion):

.selector { ... }
.header .selector { ... }
.something-else:extend(.selector) { ... }

However, in the following example, the .header .selector should get extended, but not the stand-alone (less-specific) .selector:

.selector { ... }
.header .selector { ... }
.something-else:extend(.header .selector) { ... }

So here is what I think should happen:

1) You should be able to pass ANY valid CSS selector to the :extend function. This is currently not possible and is a bug. This also prevents classical "embeds" from working:

.some-selector {
    .some.other-selector;  /* This does not work, but it should */
}

2) Every instance of the exact selector should get extended, whether it's within media queries or not. In my orignal post at the very top, I should be able to :extend(.container), but currently only the non-media query definition gets extended. 3) Unlink SASS, "more specific" instances of the selector should be left alone (eg .header .selector example above). 4) An :extend which occurs within a media query would need to smart-copy the actually css rules from other matching media queries (see my last comment for an example of this).

I would love to talk about this over phone and then summarize here. Feel free to call if you want: six one five-260-2411

matthew-dean commented 11 years ago

I'm just catching up to this discussion, just wanted to first add a +1 for one point:

"I would like to take this opportunity to say more-specific selectors should not get extended in the way that SASS does. For example, in the example below, the .header .selector should NOT get extended (in my opinion):"

Correct. .selector and .header .selector are not used in the same way. It should be an exact string match. If I'm matching .selector it should only extend (add properties to) that class and that class only. And then, of course, it's up to the browser to inherit appropriately into .header .selector.

The only way it should extend both selectors within their definitions would be something like this:

.selector { ... }
.header .selector { ... }
.something-else:extend(.selector, .header .selector) { ... }
jonschlinkert commented 11 years ago

@DesignByOnyx, I misunderstood your point because of the statement "so that all selector patterns get matched", which I interpreted as saying that you wanted SASS-like extends. I now understand that you are not saying to extend .some-selector to all selector patterns of .some-selector, but what you were saying (and had said very clearly earlier) was that when you target any give selector pattern, even if it happens to be a compound selector, like .selector-one.selector-two, it should just work. If this is indeed what you're saying (and is is the same as @matthewdl is saying), then + 1 and I agree completely. Sorry for the confusion... I got hung up on that statement, even when I tried to re-read it to make sense of the point.

I doesn't help that we're discussing two issues here. One for how :extend handles selector patterns, and one for how :extend is broken in media queries. I would suggest splitting these issues apart so that discussion on either stays focused.

DesignByOnyx commented 11 years ago

Thanks for the remarks. I think you understand me more clearly now. Sorry for the confusing wording. I agree that we should only be focused on making :extend work the way we think it should across media queries.

The other [less relevant] issue has to deal with LESS's selector engine not recognizing all valid CSS selectors... which is a separate issue but should get fixed immediately. The :extend functionality will simply benefit from this fix as would the rest of the LESS's functionality. This is the fix which I think should take inspiration from the sizzle selector engine. Just a thought.

So, now we are all on the same page. There are two different use cases to consider:

1) A simple :extend which takes place outside of a media query. This one is simple, as all instances of the selector should get extended... whether in a media query or not.

2) An :extend which takes place within a media query. This is very complex as the compiler would need to copy styles from other media queries and compile them within the given media query. See my previous comment about this scenario.

jonschlinkert commented 11 years ago

@DesignByOnyx would you mind creating another issue for :extend within media queries? We should rename this one too so it can be prioritized and others can add their thoughts.

lukeapage commented 11 years ago

regarding this point

.selector { ... }
.header .selector { ... }
.something-else:extend(.selector) { }

should not extend .header .selector

I thought the main point of extend was where you had a library of css targeting one class, and you had html you were restricted on changing with a different class.. so if I want everywhere that matches .selector to match .something-else then I do the above so that I get

.selector, 
.something-else { ... }
.header .selector,
.header .something-else { ... }

when talking about not wanting something to happen - please could you clarify why you don't want it to happen and what your particular use-case for the extend is?

lukeapage commented 11 years ago

see #1165 for a discussion of extend around media queries. I don't think we have it nailed down in this discussion exactly what it should do when you use extend inside a media query.

jonschlinkert commented 11 years ago

@agatronic

I thought the main point of extend was where you had a library of css targeting one class, and you had html you were restricted on changing with a different class.. so if I want everywhere that matches .selector to match .something-else then I do the above so that...

@DesignByOnyx

Maybe we are saying the same thing, but since @agatronic made his comment, I want to be clear:

This one is simple, as all instances of the selector should get extended... whether in a media query or not.

To clarify, you only mean "exact instances" (as you said further above) - I'm going to give examples for this below. Correct me if I'm wrong because this isn't how less.js works today, and I believe it's what Luke is asking for additional clarification on.

IMHO :extend should be fixed to act like the direct inverse of a mixin. If I use a mixin on a selector, the selector inherits the properties of that mixin, but the mixin does not extend to every instance of a selector to which it has been applied. I don't think that :extend should be applied to every instance of the selector being extended either.

So if I do this:

.square {
    width: 300px;
    .header {
        color: black;
    }
}

.circle:extend(.square) {
    border-radius: 150px;
}

I would expect to see this:

.square,
.circle {
    width: 300px;
}
.square .header {
    color: black;
}
.circle {
    border-radius: 150px;
}

not this...

.square,
.circle {
    width: 300px;
}
.square .header,
.circle .header {
    color: black;
}
.circle {
    border-radius: 150px;
}

If I wanted the last result I would do this:

.circle:extend(.square) {
    border-radius: 150px;
    .header:extend(.square .header);
}

To expand the example, currently I can do the following with :extends:

.shape {
    width: 300px;
    background: red;
    .header {
        color: black;
    }
}
.square {
    &:extend(.shape);
    height: 300px;
}
.circle {
    &:extend(.shape);
    height: 300px;
    border-radius: 150px;
}
.rectangle {
    &:extend(.shape);
    height: 150px;
}

and it compiles to this:

.shape,
.square,
.circle,
.rectangle {
  width: 300px;
  background: red;
}
.shape .header,
.square .header,
.circle .header,
.rectangle .header {
  color: black;
}
.square {
  height: 300px;
}
.circle {
  height: 300px;
  border-radius: 150px;
}
.rectangle {
  height: 150px;
}

I don't want to extend the .header, why would it do that? Now, I have to check through my entire library of LESS files to make sure nothing is extending a class I'm about add a "modifier" class to, or that class will extended as well. What I really want to get is this:

.shape,
.square,
.circle,
.rectangle {
  width: 300px;
  background: red;
}
.shape .header {
  color: black;
}
.square {
  height: 300px;
}
.circle {
  height: 300px;
  border-radius: 150px;
}
.rectangle {
  height: 150px;
}

Let's take a real-world example though, something more concrete, so you can see how this is effecting the wildlife:

.sasquatch:extend(.yeti) {
    z-index: -1;
    .fur {
        color: brown;
        background: brown;
    }
}
.yeti {
    height: really tall;
    visibility: camouflaged;
}
.yeti .fur  {
    color: white;
    background: white;
}

Today it compiles to this:

.sasquatch {
    z-index: -1;
}
.sasquatch .fur {
    color: brown;
    background: brown;
}
.yeti,
.sasquatch {
    height: really tall;
    visibility: camouflaged;
}
// So far, so good. All true.
.yeti .fur,
.sasquatch .fur {
    color: white; // Wait, what?! okay this is not good
    background: white; // sasquatches need dark backgrounds to hide. damn you cascade, and damn you extends! 
}

Too bad we can't do this: .sasquatch:extend:not(.yeti .fur) {}

Because now our friend is in danger:

.hunter:target:only-of-type[class*="-hominid"] {
    // Why must he be this way? What's wrong with hunting less endearing species, like pandas?
    content: "Success" attr(data-status);
}

And then:

.sasquatch:last-of-type {
    content: "unfortunately";
    display: never again;
}

No more sasquatches. Shame. All because of the unintended consequences of extending all instances of a selector.

lukeapage commented 11 years ago

I think I see your point. In your shape circle example, If I wanted header to be black I would just set the class name to "shape circle".. where as extend would be useful If I wanted to extract shape's properties without including any other sub properties?

I definitely think we should limit things with extend as much as possible.. that way we can see peoples reaction to it and the use-cases that come up and extend (ha) the behaviour from there.. if we go all out from the start we might end up with something useless.

jonschlinkert commented 11 years ago

@agatronic yes! exactly. However, I also think that a comma separated list of values should be allowed (which I think was in my original proposal, #509 as well). Like this:

.oval:extend(.shape, .circle) {
    height: 150px;
}

I like it because it still follows CSS-convention - or at least what is proposed for the :not and :matches selectors in CSS4, and it compensates for not matching every pattern of a selector by default:

CSS4 proposal:

E:not(s1, s2)
E:matches(s1, s2)

Found here: http://dev.w3.org/csswg/selectors4/

DesignByOnyx commented 11 years ago

@jonschlinkert - I am actually using :extend because of all of the nested styles that get extended. In reference to your .shape .header example, I personally would want the nested .header to be extended as well (don't hate me yet). I feel this way because of the declarative nature of the styles. Let me explain.

.shape {
    .header { ... }
}

.circle:extend(.shape) { }

Unlike you, I would* expect and prefer to end up with this:

.shape, 
.circle { ... }

.shape .header,
.circle .header { ... }

I expect this because of the way I wrote the LESS styles. I am currently leveraging this functionality and I really like it. Now, consider the following declaration:

.shape {
    .header { ... }
}

.shape .footer { ... }

.circle:extend(.shape) { }

I would not expect the .shape .footer styles to be extended because it is declared as it's own separate thing. The compiler should treat the above code like this, in my opinion:

.shape,
.circle {
    .header { ... }
}

.shape .footer { ... }

I think this strikes a balance for the you's and the me's out there. Choose your poison, essentially.

lukeapage commented 11 years ago

:extend

and

:extend-all

?

matthew-dean commented 11 years ago

Agree completely with @DesignByOnyx's interpretation of syntax and of LESS convention. If I extend a block, then by extension I extend its nested blocks. I defined them as a single unit. The entire set is enclosed in curly braces. Therefore, they are one declared definition, and I think this would be intuitive to most LESS devs.

Along the same lines, I also agree that it does not imply that I mean to extend a similarly classed block, just because it happens to partially contain the class name. If you think about it intuitively, it shouldn't. If I extend something like .header, then I should not extend .page .header for the simple reason that those styles will already be inherited because of basic CSS inheritance. Therefore, to add the properties to both will always be unnecessary duplication. :extend should always, always be exact selector matches.

I also agree and expect @jonschlinkert's usage, that :extend would behave like other matching selectors, and allow comma-separated values. To me, that's implied by using the syntax at all.

-1 to :extend-all. Let's not repeat @import and @import-once, where we end up with multiple syntax additions because we can't agree on behavior of :extend. I'd much rather see us arrive at consensus, rather than make more additions to LESS syntax, which seems to have already doubled in the last year. Don't get me wrong, it's great that there's so much active development on LESS now, but please always keep in mind that many choose LESS over other CSS pre-processing languages because of its simplicity and low learning curve.

jonschlinkert commented 11 years ago

Agree completely with @DesignByOnyx's interpretation of syntax and of LESS convention.

It's difficult to debate our interpretations of syntax and LESS convention in general, but since I'm the one that proposed this specific syntax as well as the intended implementations, we'll just have to agree to disagree on this one ;-)

When I proposed the :extend syntax I attempted to demonstrate that a primary advantage of this syntax over SASS's is that we could choose specific selectors to extend. Just because mixins work the inverse of the way that you're describing (inheriting nested blocks) doesn't mean that extends should work that way. I proposed that they should not, and because extends are not specific enough as they are, I haven't used them in practice once, nor will I until they become less dangerous. As I see it, this could be resolved by 1) allowing a comma separated list of values, and 2) only extending the specific iterations of selectors listed. Because:

  1. there is little advantage to the powerful syntax we have without allowing a comma separated list of selectors
  2. there is no advantage to a comma separated list of values with the current implementation, and in fact
  3. If we implement according to how @matthewdl and @DesignByOnyx are suggesting, we should not allow a comma separated list of values. It's too confusing, it might conflict with or duplicate a nested selector which has already been defined, and it makes it even more difficult to know what has or hasn't been defined.

@matthewdl, what is the resulting CSS you would expect if I did this:

.shape {
    width: 300px;
    background: red;
    .header {
        color: black;
    }
}
.shape .header {
  font-size: 16px;
}
.square:extend(.shape);
    height: 300px;
}

And what would you expect if I did this:

.shape {
    width: 300px;
    background: red;
    .header {
        color: black;
    }
}
.square:extend(.shape) {
    height: 300px;
}
.circle:extend(.square) {
    border-radius: 150px;
}
.rectangle:extend(.square) {
    height: 150px;
}
.oval:extend(.circle) {
    height: 200px;
}

This example is important, because it shows a very strong and practical use case for the :extend feature, which we have not seen from any of the other points being made. Here is the resulting CSS based on the implementation of extends today, and it would be the same if we follow @matthewdl and @DesignByOnyx's solutions:

.shape,
.square,
.circle,
.rectangle,
.oval {
  width: 300px;
  background: red;
}
.shape .header,
.square .header,
.circle .header,
.rectangle .header,
.oval .header {
  color: black;
}
.square,
.circle,
.rectangle,
.oval {
  height: 300px;
}
.circle,
.oval {
  border-radius: 150px;
}
.rectangle {
  height: 150px;
}
.oval {
  height: 200px;
}

There are multiple problems here.

There were points made about inheritance and intuition, and what I think is intuitive would have been to extend only the specific selectors I extended, which means that I could extend the shape without the superfluous headers (and "extends chaining" is another issue entirely).

With nested selectors being extended, we would need to stop nesting our styles to target a single selector, which negates a primary advantage of LESS over vanilla CSS. I don't think that's intuitive, and it's the opposite of what I personally would expect.

However, I will concede that in a worst case scenario, if a compromise had to be made, I could accept the extending of nested blocks too, but only if I couldn't convince you not to do it otherwise, if comma separated selectors were not allowed, and because it seems to be intuitive to others based on comments. But to those who like the idea of extended nested selectors, please keep in mind that their is a significant difference between mixins and extends. Mixins do not show in your compiled CSS unless they are used. But selectors, extended or otherwise, do show up. And when :extends are chained, styles will multiply and compound out of control quickly. This is something I find very distasteful about SASS.

And imagine 3rd party libraries using extends as described. I often see developers (and libraries, javascript plugins, etc) wrap entire stylesheets, and nest dozens upon dozens of classes in a single id or class. How could you (or they) extend anything specific within that block? You could't, you would have to refactor the code entirely, which negates the value of extending in the first place. With my proposal you would be able to do :extend(.parent .child) and nested or otherwise you would extend that selector.

IMO the best solution is to allow a comma separated list of selectors AND to only extend the selectors specified. With the current implementation, and with the implementation described by Matthew and @DesignByOnyx, it would not be possible to get the styles from a single selector if that selector had anything nested within it. In my opinion this is a terribly restrictive and poor implementation of the spec. If a comma separated list of selectors was allowed I just can't wrap my brain around the argument for extending nested selectors. Why not just use the selector you're extending if you want to extend all of it's manifestations? And if you're argument is that you wold only want to extend "a couple of manifestations" or "just the ones that are nested", then use a comma separated list of selectors.

lukeapage commented 11 years ago

@matthewdl I'm very confused as to what you are saying - you appeared to be supporting both arguments? I read it like you were in favour of only matching the specific selector use but @jonschlinkert seemed to interpret it as the opposite?

Along the same lines, I also agree that it does not imply that I mean to extend a similarly classed block, just because it happens to partially contain the class name. If you think about it intuitively, it shouldn't. If I extend something like .header, then I should not extend .page .header for the simple reason that those styles will already be inherited because of basic CSS inheritance. Therefore, to add the properties to both will always be unnecessary duplication. :extend should always, always be exact selector matches.

If you are extending .header then it means your dom element will not have the class "header" - so how will it already get those styles because of css inheritance?

@DesignByOnyx's point about having different behaviour for a less block as a css block is, I think incredibly dangerous.. If I refactor some less from

.a .b {
  color: black;
}

to

.a {
    margin-left: 3px;
    .b {
        color: black;
    }
 }

I would not expect a difference in behaviour.

I don't know really what to do because there will be multiple use-cases that everyone wants to solve with extend. I thought the whole point of extend was that it would be able to replace a single class or a single node type in complex scenarios.. which is something that you cannot do with mixins... all of these examples seem to be about really simple cases where either you should just call .shape(); or you basically want to be able to get .shape(); but without the inner classes..

E.g. I thought it was more about

li > a.blah:extend(.shape) {
}

now li > a.blah will basically be treated as a shape. (and there is only any point in doing that if .shape is used in complex selectors and not just .shape { ......

Also I don't know how any of you expect the above to happen when myself and a few contributors are the only people actually contributing code to make things happen? From a code perspective its going to be alot difficult to support @jonschlinkert suggestion, though I guess it isn't impossible.

rather than make more additions to LESS syntax, which seems to have already doubled in the last year. Don't get me wrong, it's great that there's so much active development on LESS now, but please always keep in mind that many choose LESS over other CSS pre-processing languages because of its simplicity and low learning curve.

@matthewdl I cannot believe you think additions to the less syntax has doubled in the last year. There have been changes but I think you'll find that apart from adding functions and fixing existing features (e.g. (~"@{selector}") and maths in parenthesis - which are changes only) there have basically been no new features in the last year - just me fixing hundreds of bugs as we agreed to get that out of the way first.

jonschlinkert commented 11 years ago

@agatronic just a quick comment, I sincerely want you to know how much I appreciate your contributions. Not just saying this... If I had your talent, I'd be helping right alongside you. My skills rest in other areas so hopefully I can continue to help out there.

I think the "speed of implementation" is something that you or anyone else who contributes code decides based on your own schedule and being pragmatic. Sometimes these discussions get going with a lot of momentum, and maybe that leads to some perception of urgency, but it shouldn't necessarily. I'm appreciative of everything you and @matthewdl and other contributors have done (and are doing), and I personally don't feel any sense of entitlement when it comes to your time.

I will jump back on later and add some comments on the issue itself.

jonschlinkert commented 11 years ago

Oh, and perhaps there is just a communication issue. This might be something we eventually need to discuss over skype with a webex or something. I think the issue is complex enough that real world examples need to be reviewed, and it would be much easier to demonstrate things real-time.

Soviut commented 11 years ago

That sort of discussion and/or presentation would do well to be recorded and presented on YouTube. I've been trying to keep up with all the talk here, but it's difficult. Seeing live examples of proposed ideas from those involved would go a long way to clearing up some of these more complex issues.

jonschlinkert commented 11 years ago

@Soviut sounds like a reasonable request, I think it would be a matter of prioritization and scheduling. anyone else have thoughts on this?

matthew-dean commented 11 years ago

And, I should apologize in this thread too. Mostly to @agatronic. I think I said things more dickishly this week while I had the Plague. I love you all. Please accept my love. PLEASE ACCEPT IT. ..... Wait, I think now I may have gone too far the other way......

Moving on.

I think discussion of the :extend syntax has exposed some differences in how we (internally) interpret LESS code. That is, how we look at Less blocks and sort of mentally convert into CSS.

One small point:

As to this:

.a .b {
}

...being different from this:

.a {
  .b {
  }
}

...Yes, I would expect them to be different in relation to :extend. Less already treats them differently internally as not the same, even if they have the same CSS result. They are stored differently and have different scoping properties.

There's a lot of interesting edge case questions, but specifically about this kind of block:

.a {
  .b {
  }
}

If I write: .c:extend(.a) { }, mentally, this is what I'm seeing:

/* Step one */
.a {  <-- I, the Less parser, have found a match! Let us extend!
  .b {
  }
}

/* Step two */
.a, .c {  <-- updated Less definition
  .b {
  }
}

I don't know any other way to see this. It's all good and fine to say you don't want the .b class to be extended, but...... what else are you doing except the above? Are you not extending .a? Does that not imply the entire definition of .a? If you didn't want to include .b, why did you put it in there and then extend the parent class? I have a hard time seeing it from a different perspective. What else would the extend keyword imply?

Now, if that's an undesired result, that's fair, and we could address that problem. But, that seems like the expected result, to me.

matthew-dean commented 11 years ago

@jonschlinkert Specifically to your examples, if you're following what my expectation is, and if that truly would be the result, so be it. To channel @cloudhead a bit here, if you write a lot of weird stuff, you'll get weird results. You extended a bunch of times, and got a bunch of (perhaps unwanted or unneeded) output CSS. Less won't solve that. It doesn't make the result necessarily unexpected, is what I'm saying. It could be a simple case of, "Sure, that's how it works, you might have to approach in a different way."

As I said, it doesn't mean we couldn't try to also solve the problem of cludgyness. But what I'm trying to help solve is what is the expected result. And what you may be trying to solve is what is the most useful result. The two are not mutually exclusive, but I'd be curious if you'd agree with the above, about what it is we're extending. To me, it's the LESS definition, but I think some people are thinking of it as extending the CSS selector.

jonschlinkert commented 11 years ago

But what I'm trying to help solve is what is the expected result. And what you may be trying to solve is what is the most useful result.

Seems like some good insight into how we're coming at this from different angles (which is a great thing!). I guess if I describe how I'm viewing this, it's not so much that i'm looking for the "most useful result" as it is "the most predictable result will lead to the least damaging result". With extends, the more specific you are required to be, the more likely you are to see predictable results in your code.

Mixins can be used hundreds of times throughout your code, and the net impact to the compiled CSS should always be incremental, linear, and directly correlated to the number of times each (type of) mixin was applied. In other words, if you use a box shadow mixin, you know that the resulting CSS will increase by the amount of code in that mixin. But extends are not like mixins at all, they're like gremlins. They multiply, chain and compound.

So what about this for a compromise? 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 only 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 itself doesn't show up in the compiled result.

If "extending mixins" was implemented, and extends allowed a comma separated list of selectors that would be pretty powerful.

jonschlinkert commented 11 years ago

Thought I would link to this as well, it's worth reading and it's short: http://learnboost.github.com/stylus/docs/extend.html

Stylus does allow extending nested selectors... if they are specified exactly as I'm describing.

Also, I urge anyone interested in this topic to just do some research on extends with other languages, like Stylus, SASS and SCSS (which were actually implemented differently). And I also highly recommend that you do some research on the user groups for those languages, Stack Overflow, and do a google search for blogs about extends (pros, cons, usage, advantages and disadvantages). I'm not trying to browbeat hear, I just want it known that extending nested selectors is viewed as one of the bad parts of other implementations. It's not a secret and it's not difficult to find articles on the topic (I linked to one earlier in this thread). If you get rid of extending nested selectors (unless you specify one!), all the other things about extends are great. (also, if you happen to read about "chaining extends" and "extending nested selectors", many bloggers confuse the two and use the terms interchangeably. They aren't the same, but we can come back to that if need be).

matthew-dean commented 11 years ago

That's interesting, and we could extend mixins, but shouldn't we solve the problem we're on first? Extending selectors?

Yes, there are many damaging and cumulative results possible with an extend syntax, which is why I'm glad it's getting a lot of debate. I'm not debating that. In a simple example, like above, I'm asking myself what I would expect it to do, and what I would want it to do. While the output for complex examples seem verbose, you are correct in pointing out that the current model, mixins, leads to even more repeated code. That's the reality of CSS. If I want a variety of classes, or widgets (and that's one might view a nested LESS block, a widget) to share common properties, then I need comma separated values on each of those blocks of classes. Or I simply repeat them verbatim in another block, which is what mixins provide.

I think what you're uncovering is precisely right which is what users of other libraries with extend have discovered, that it can lead to giant blocks of results. But that doesn't mean it's an incorrect implementation. It also can be problematic because it can effectively break your inheritance model, simply by moving your definitions higher.

All of those may simply be arguments against including an extend syntax at all. I was actually originally happy that Less didn't have an extend syntax, because it didn't import that pile of pain that I was seeing talked about in other libraries, where users had to work so hard to understand what was happening or why something didn't inherit. (Not that they all did; obviously people who got it wouldn't be complaining.)

The Less mixin model, while often leading to piles of repeated output, does so in a way that mimics CSS. Stuff doesn't move around. It maintains inheritance order. But, it's (understandably) uncomfortable for people who want their stuff much more object-oriented, or want fewer selector blocks for the browser to interpret (for better performance).

TL;DR - :extend is one powerful tool, that can lead to ill results, regardless. I respect wanting to mitigate it's damage, but I worry that we're trying to solve the problem of bad coding (protect developers from themselves), or solve the problem of CSS, which requires so much redefinition. Granted, LESS is, in some ways, trying to "fix" CSS, but at the end of the day, some things in CSS are (currently) unfixable.

So, let's keep it to a smaller scope. Back to my original question, and my original example. If this is a LESS extension, then in your mind, what "object" are you seeing it extend? The LESS definition (selector block), or the CSS selector? If it doesn't apply to the whole nested block, then I think we need to explain why. That's all I'm saying. I think the scope of this needs to stay focused.

jonschlinkert commented 11 years ago

I don't think the suggestion to extend mixins as an alternative to extending selector blocks is off-topic at all. I think it's a strategy for an effective resolution that would yield results with extends that are more desirable than the current implementation. In fact, since I'm suggesting that when you extend a mixin, you extend it's entire block, this ought to be considered as an alternative to extending "regular" nested selector blocks. That would allow you to actually plan for the results that extends would yield. But as extends are currently implemented, if you forget that a regular old selector is extended and you nest some code inside the selector block, guess what... that's getting extended too. And you might never even notice it in the compiled CSS. That sounds like a bug to me.

But that doesn't mean it's an incorrect implementation.

But yes, it does mean precisely that, because Less.js is the last to show up at this particular race (extends) and we have an opportunity to learn from the experiences of others (SASS and Stylus), rather than repeating their mistakes - which have been written about over and over again. We have an opportunity to implement more conservatively, the "LESS way", not the SASS way or the Stylus way (although admittedly we would be probably be implementing more like Stylus here). I'm not a fan of the terseness of Stylus, but at least Stylus learned from SASS's failures with extends and implemented the way it should have been.

All of those may simply be arguments against including an extend syntax at all.

I would prefer not to throw out the baby with the bathwater.

The Less mixin model, while often leading to piles of repeated output, does so in a way that mimics CSS. Stuff doesn't move around. It maintains inheritance order.

Not to be a $#%, but the output of every feature implemented in LESS should be CSS, not mimic it. But the problem with :extends is that they do in fact "move stuff around", which can effect inheritance order. This is completely acceptable and predictable when extending only a specific selector or even a specific nested selector, but it's not acceptable when selectors that are not specified are extended as well. This has unintended consequences. This is why SASS @extends have been described as "fools gold". They appear to be useful because you might believe you are only extending the selector that you're, well, extending, but alas - SASS also extends other selector patterns that branch off of that selector. Extends should be used as a DRYer alternative to mixins. Please think about what I'm saying here. If extends were implemented as I'm suggesting, they would be the DRY version of mixins!

I worry that we're trying to solve the problem of bad coding (protect developers from themselves), or solve the problem of CSS

Maybe that is something we should get feedback from @cloudhead on. But in my view, LESS is solving the problems of effective and efficient coding. After all, we cannot help or hinder the syntax of CSS - we can only help or hinder the developers writing it. If we keep helping, we'll stay relevant and continue to grow. If we implement as you and @DesignByOnyx are suggesting, there are so many unintended consequences I'm having a hard time coming up with any beneficial use cases of that implementation at all. Meaning, yes, you would get the "good parts" of extends, same as you would get from my suggestion. But you would also get the unwanted cruft, which is what I'm suggesting we avoid. I'm really not intending to be confrontational here, I think it would help to have some actual use cases, with real-world code, to demonstrate how your suggested implementation of extends would result in code that was more effective OR more efficient that it would have been using some other existing feature.

If this is a LESS extension, then in your mind, what "object" are you seeing it extend? The LESS definition (selector block), or the CSS selector? If it doesn't apply to the whole nested block, then I think we need to explain why.

That's fair, and I've attempted to do that (clearly unsuccessfully). But in my view, extends should use the CSS definition of a selector, because:

  1. It yields vastly more predictable results
  2. It yields more conservative results, in LESS tradition
  3. It stays closer to the native language, in LESS tradition
  4. The conservative approach will always be easier to improve and build upon later, rather than backtracking. Even if it is more difficult to implement the feature initially.
  5. It should be more intuitive to everyone who writes CSS, regardless of their familiarity with LESS. It is super straightforward to explain that when you do this: :extend(.shape .header), you will ONLY extend .shape .header, regardless of whether or not .shape .header is inside a nested block. No one should have to think that hard.
.shape {
    width: 300px;
    background: red;
    .header {
        font-size: 80px;
    }
}
.square:extend(.shape .header) {
    height: 300px;
}

The result would be:

.shape {
  width: 300px;
  background: red;
}
.shape .header,
.square .header {
  font-size: 80px;
}
.square {
  height: 300px;
}

And if I want to extend the base .shape, I would add that to the comma separated list of values: .square:extend(.shape, .shape .header)

If this doesn't make sense, I might need some help understanding why it makes more sense to yield a selector that I didn't specify. Like if I was to do :extend(.shape) and it extended .shape .header as well. (clearly this doesn't do it justice either, there could be dozens of other selectors that got extended, despite the fact that they weren't specified).

It would help if we all say what's actually on our minds here. I'm giving solid, real-world examples and reasoning in my arguments. I think we need to really look at the code, and we need to see what's happening under the hood and make a decision that is informed by practical use cases. The original perceived usage of extends was undoubtedly influenced by SASS's implementation of the feature. But that's a mistake, that model is flawed, and we have a chance to improve upon it and do things the LESS way.

jonschlinkert commented 11 years ago

BTW, there is nothing but love from me for the rest of my fellow contributors here... this is a good debate

Soviut commented 11 years ago

Thanks for the succinct example. That makes perfect sense to me and would be the behaviour I'd expect.

matthew-dean commented 11 years ago

I think those are sound arguments. One of the things I'm not sure about is

.square:extend(.shape .header) { }

applying to:

.shape {
    width: 300px;
    background: red;
    .header {
        font-size: 80px;
    }
}

My preference would be that :extend only applies to less-defined selectors, not resulting CSS ones. The parser doesn't actually "know" the resulting CSS until it makes it with the toCSS() functions IIRC. I suspect there's too many cases where we'd make work harder for ourselves, and in a different way, but the same idea as you, I'd like to keep matches conservative e.g. exact text matches. Its easy to document / understand when looking at code, and less likely to lead to unexpected results.

So, I would have it only apply to:

.shape .header {
}
jonschlinkert commented 11 years ago

It would be good to get @agatronic to weigh in again, but I think what you are saying is reasonable @matthewdl. I think a more conservative approach is favorable over the alternative. I do think that the nested-block style and inline style should both be valid when doing something like this .square:extend(.shape .header) { }, but I think this particular thing might be a matter of taste, and to me it's okay if we implement without block style if that's what is called for. We can always re-evaluate implementing it later after receiving more feedback and stories of real usage from the community.

By the way, I want to correct something that I've said multiple times about SASS lacking features in extends. The correct information is that both the SASS and SCSS implementations of @extend do in fact support a comma separated list of selectors. I stated that "Less.js' :extend() syntax would allow that, and theirs @extend wouldn't". I was wrong. I probably should have said, "their implementation of extend can't support a comma separated list of selectors gracefully". In any case, SASS has some awesome and powerful features, I didn't mean to slam on them for no reason, and I just wanted to man up.

matthew-dean commented 11 years ago

To allow a comma-separate list is intuitive. I think we're on the same page there.

DesignByOnyx commented 11 years ago

I would like to agree with @matthewdl on the extending of less-defined selectors. This is what I tried to describe above, and I think it satisfies the widest array of solutions. To me, its a very declarative way of writing LESS code and defining blocks which should get extended while still having the ability to separately declare nested styles which should not get extended.

So continuing with our ongoing example:

.shape {
    width: 300px;
    background: red;
    .header {
        font-size: 80px;
    }
}
.shape .header {
    background: blue;
}

There are 3 different scenarios/outcomes I would expect. 1) .circle:extend(.shape) - This would result in extending the entire shape block above, but not the .shape .header declaration. At any point in time, I could move the background: blue into the .shape block so that it too gets extended. This allows me a great amount of flexibility in defining what gets extended and what doesn't.

2) .circle:extend(.header) - Nothing with regards to this example should get extended in my opinion. I have a feeling this is going to be a hot issue. My argument is that .header styles should get extended, but context related styles (eg. .shape .header) should be declaratively and intentionally extended separately.

3) .circle:extend(.shape .header) - This would only extend the style block with the background: blue.

jonschlinkert commented 11 years ago

@DesignByOnyx. I don't think there is any confusion at all on what you're suggesting should happen. I just disagree.

My opinion is that, as a habit (and rule of thumb), it would be good to always implement new features using sensible defaults, and then build upon those defaults with options. Here I think the sensible default should be a conservative implementation, and afterwards we look at adding other behavior. I really don't hate the idea of implementing "catchall" features that will solve all of the world's problems, because the "product never meets the pitch". I'd rather have the feature do one thing really well without having to over-think aspects of my code that are really not important for any other reason than to avoid problems with the "nested blocks feature" of extends. In other words, if we implement your way, in order to avoid unintended consequences, everyone who wants to use extends will be forced to rethink how they have written their nested blocks before they can begin using extend at all. With my suggested approach, no one would have to change anything about their existing code to use the feature, and the results would be as expected. Agreed that I wouldn't "get the bonus value meal side items" after only ordering a burger, but that's cool with me. I like getting exactly what I ordered.

I think it satisfies the widest array of solutions

That's part of the problem with your argument. It needs to solve one problem really well, and not be "an array of solutions looking for problems to solve".

Point by point:

  1. I disagree. I gave extensive reasoning and practical use cases against your argument above. A goal of LESS, as with declarative programming in general, should be to try to minimize or eliminate side effects. Rather than implement features one might need, let's implement only the features we definitely need, in a way that accommodates change, and with minimal abstraction. There is no getting around it, what you are describing would in fact maximize unintended side effects, and thus far only negative ones have come to mind. Furthermore, I think if any beneficial results of extending beyond the selector specified to other selectors within its nested block were to occur, it would be an exception, not the rule. If you still disagree, it would help the discussion if you would provide a gist or something with real examples or even use cases where your expectations ("built in side effects") would be a desirable outcome. I understand that mixins work the way you're describing, but my opinion differs in that I don't think the same behavior should apply to extends. Not to mention it's something we would have to educate CSS folks on, "Not only will we extend the selector you wanted to extend, but (like mixins), we'll extend selectors nested within the block of the selector you specified as well." Why? Seriously, why is that advantageous. It's clear why it's advantageous with mixins, it's not clear with extends. Imagine for a moment that mixins not only passed on their properties, but they also inherited all the properties of each selector that used the mixin, and then the mixin also passed on the properties it inherited from other selectors. It's not a one to one comparison, but it's close to what you're suggesting with extends. If you don't agree, spend some time researching "extends chaining" (which IS a desired result by the way).
  2. We're in agreement here. Since the selector isn't .header, it's .shape .header, I don't think it will be a hot issue, I think anyone who understands basic rules of CSS would have to agree with your point here.
  3. @matthewdl agrees with you on this. I don't, but I'm willing to accept it. There are concrete, measurable reasons not to do what you're suggesting in the first point, but I think this one comes down mostly to perception and preference. I usually resolve things like this (for myself) by trying to imagine writing the documentation for it, and this isn't intuitive in my opinion, so I expect some people will view your suggestion as a bug. But we'll see.

Another thought regarding the first point. What if you extend a nested selector and need to change something about it at some point, and maybe you don't want everything extending that selector to be effected? Keeping in mind that that ANY selector can be extended, and when a selector is extended there will be no indication on the selector itself that it was extended - this is abstracted to the extending selector, and that abstraction is compounded when a) nested blocks are extended and b) extends are chained (which again is a desirable effect of inheritance, that is unless nested blocks are extended as well). What are your thoughts on best practices here? With mixins, it's easy and there is little abstraction between the mixin and selector using it; if you need to find all the instances of the mixin being used, ctrl+f. Not so with extends.

Last thought, this feature is big enough, and powerful enough that there absolutely is potential for enhancing the feature further after implementation. Many SASS proponents believe that extend is the most powerful feature of the SASS language, and I believe we will find it to be the same for LESS. So I don't think it will be distasteful at all to consider enhancements like what @agatronic suggested when he mentioned :extend-all, and perhaps @matthewdl would have more of a palette for the idea under those circumstances as well. I'm not sure what the actual syntax or use case would be until their is a need for it, but the point remains that this kind of enhancement is easier to do when the feature being enhanced is well-defined and narrow in scope. Then the enhancement doesn't feel like things are getting out of control as they have with the @import statement.

DesignByOnyx commented 11 years ago

@jonschlinkert - Thank you for your thorough and thoughtful comments, and I'm sorry for beating a point.

I would like to make a comment which I earlier refrained from making... but you have now prompted me. This is in response to your comment:

... if we implement your way, in order to avoid unintended consequences, everyone who wants to use extends will be forced to rethink how they have written their nested blocks before they can begin using extend at all. With my suggested approach, no one would have to change anything about their existing code...

Having used the :extend feature in a real world application which will be launching soon, I can say that the feature should not be used "willy nilly" just because the new version of LESS has the feature. Using this requires some level of planning and may require restructuring of code with the intent of leveraging this new feature the way it's supposed to be leveraged. I am of the opinion that implementers should be forced to rethink their style blocks before using the feature, and it would only be "responsible parenting" on the part of the LESS team.

What if you extend a nested selector and need to change something about it at some point, and maybe you don't want everything extending that selector to be effected?

These types of situation are simply going to come up even with advanced users. Users will simply have to refactor some of their styles much the same way we refactor method footprints, variable scopes, and any other "oh shit, the old way no longer works" kind of situations.

So you ask for a real world example, and here you go. My main goal with the :extend feature was to remove as many layout-related classes from my markup and simply have an extended set of rules in my stylesheet:

.clearfix {
    *zoom : 1;
    &:before,
    &:after {
        display: table;
        content : ""; 
    }
    &:after {
        clear : both; 
    } 
}

.grid-wrap {
    letter-spacing: -0.31em !important;
    word-spacing: -0.43em !important;

    > * {
        letter-spacing: normal;
        word-spacing: normal;
        display: inline-block;
        vertical-align: top;
        .box-sizing(border-box);
    }
}

[role="main"]:extend(.clearfix) {
    background-color: #fff;
}

.artist-list:extend(.grid-wrap) {
   text-align: center;

    > li { width: 33.3%; text-align: left; }
}

.columns:extend(.grid-wrap) {
    > section { width: 65%; margin-right: 5%; }
    > aside { width: 30%; }
}

You can see from this example why I am advocating that the nested selectors be included by default. I am almost inclined to say that I wouldn't even use the feature if I had to extend every nested rule individually. The last argument that I would make to this point is if I want to add another nested rule to the .grid-wrap block, I would be extremely easy to add it once and be confident that it's getting extended everywhere. Otherwise I would be forced to find every instance of :extend(.grid-wrap) and include the new nested rule... which is a royal pain and very error prone (eg. forgetting a nested rule, mistyping a nested rule, renaming nested rules in the future, etc).

jonschlinkert commented 11 years ago

These are good examples, and they are exactly the kind of thing I was thinking when I suggested the idea of extending mixins. Grid systems and "utility" classes, like clearfix, are really an ideal fit for extending mixins. Seems like you would have the best of both worlds, but based on your reply maybe I'm still missing something.

Worst case, my approach still allows you to accomplish your goal:

[role="main"]:extend(.clearfix, .clearfix:before, .clearfix:after) {
    background-color: #fff;
}

.artist-list:extend(.grid-wrap, .grid-wrap > *) {
   text-align: center;

    > li { width: 33.3%; text-align: left; }
}

.columns:extend(.grid-wrap, .grid-wrap > *) {
    > section { width: 65%; margin-right: 5%; }
    > aside { width: 30%; }
}

And actually, there is something I haven't thought of.... "extends chaining" means you can extend something that is extending something else. Unless I'm missing something, you might be able to get around the limitations by doing something like this:

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

[role="main"]:extend(.clearfix) {
    background-color: #fff;
}

Yes, it's one more step. But it could easily be used as a way of intentionally extending a nested block to get around the default of only extending the specified selector.

I'm kind of tired, so let me know if there is some big hole in the logic here... Thank you as well for the comments.

lukeapage commented 11 years ago

@matthewdl

My preference would be that :extend only applies to less-defined selectors, not resulting CSS ones. The parser > doesn't actually "know" the resulting CSS until it makes it with the toCSS() functions IIRC. I suspect there's too many cases where we'd make work harder for ourselves, and in a different way, but the same idea as you, I'd like to keep matches conservative e.g. exact text matches. Its easy to document / understand when looking at code, and less likely to lead to unexpected results.

hrmm.. that's not actually so true. Less compiles into an ast of nodes to describe the selector and then a complex algorithm takes the father and mother selector and creates a new selector.. there is no text matching apart from at the fine grain level of comparing an element or class name or combinator - this combining happens in evaluation and so does the extend. We have to compare the nodes, not the text.. the only question is will we compare after the selectors have been merged or before.. or indeed a mixture of both. I am not 100% but it could even be harder to do what you suggest than easier.

lukeapage commented 11 years ago

@jonschlinkert I like your extend chaining example.

jonschlinkert commented 11 years ago

thanks :+1:

DesignByOnyx commented 11 years ago

Great discussion.

@jonschlinkert - So besides the verbosity and difficulty of maintainability of your examples, the actual implementation is a bit confusing. You provided this code:

[role="main"]:extend(.clearfix, .clearfix:before, .clearfix:after) {
    background-color: #fff;
}

From this, I would expect to end up with this CSS:

[role="main"],
.clearfix {
    zoom: 1;
}
[role="main"],
.clearfix:before,
.clearfix:after {
    display: table;
    content : ""; 
}
[role="main"],
.clearfix:after {
    clear: both;
}
[role="main"] {
    background-color: #fff;
}

Sticking to your approach of extending every nested selector separately, I would instead expect to write my code like this and end up with the proper results:

[role="main"]:extend(.clearfix) {
    background-color: #fff;

    &:before:extend(.clearfix:before) { }
    &:after:extend(.clearfix:after) { }
}

Again, I think either method here is verbose and inefficient and likely to deter people like me from using the feature. My example above is working in a real project today, and it feels and works very nice to someone who has been doing front end dev for 10 years.

With regards to chaining, you bring up another issue I was kind of hoping to avoid. Having been using this feature on a real project, I can say that there are some good use cases for chaining but it dramatically increases the complexity and confusion of the code. I would personally avoid chaining and recommend against it, especially in a team environment, but I really appreciate an honest attempt at providing a solution to this problem. If this ends up being the solution, the chaining would be a part of my core LESS stylesheets and I would only document/expose the final .clearfix class to my team for extending while leaving the .clear declaration as sort of a "private" class which the rest of the team is instructed not to use or extend. This is all just to minimize confusion. Again, I would expect to write the code as such (as your example doesn't make sense to me):

.clear {
    *zoom : 1;
    &:before,
    &:after {
        display: table;
        content : ""; 
    }
    &:after {
        clear : both; 
    } 
}
.clearfix:extend(.clear) {
    &:before:extend(.clear:before) {}
    &:after:extend(.clear:after) {}
}
[role="main"]:extend(.clearfix) { }
matthew-dean commented 11 years ago

@DesignByOnyx You put more elegantly what I was trying to say. I actually think that extending the whole block is the more maintainable / conservative option, and the clearfix example is quite a good real-world one. @jonschlinkert, while defining an extend for each nested selector may work for the clearfix example, I can tell you a number of times where I've had projects where we've had deeply nested selectors that it would be a maintenance nightmare to create extend statements for every nested selector, plus, it creates more work. Every time you add a selector to the root block, it wouldn't be automatically added to your derived class, which defeats the purpose of extend (in those use cases).

Plus, imagine if you have several widgets which extend a root object, like, for example, 20 classes which extend .clearfix. If I add another nested selector to .clearfix, that means I have to copy 20 extend statements to my other classes, which violates the DRY principle. That just seems like a completely untenable solution.

It's not that I don't get wanting to sometimes opt out of extending any nested selectors. But the solution to that seems so easy (moving the nested selectors outside of the block being extended) vs. the difficulty / unmaintainability of adding 20 extend statements (when nested selectors aren't extended), that that's why I stand by that position: that extending the entire block is far easier, and more maintanable for devs.

DesignByOnyx commented 11 years ago

I would like to propose a new solution:

Extending a .standard-css-selector will only extend that one selector and none of it's nested selectors. Extending a .mixin() will extend all nested selectors and receive the other benefits of extending mixins

This is easy to document and lets the developer choose his poison. I kinda like this because I could see something like this in my code... which would be very useful in my current project:

.some-selector:extend( .some-nestest-group(), .some-single-selector ) { }
jonschlinkert commented 11 years ago

I would like to propose a new solution:

@DesignByOnyx, maybe it's wording that we're getting hung up on, but hopefully we're about to break past that. The "new solution" that you're proposing is exactly what I've been advocating since early in this thread, and virtually to the letter. (although I do have one question I'll ask below)

Extending a .standard-css-selector will only extend that one selector and none of it's nested selectors. Extending a .mixin() will extend all nested selectors and receive the other benefits of extending mixins

Agreed, can we embrace in a long-distance man hug and call this a victory? Okay, no man hug, but that's precisely what I've been saying. Maybe it threw you off above when I said "Worst case, my approach still allows you to accomplish your goal", and then I gave an example that was intended to be a worst case scenario per the prior statement. I'm not sure, maybe you thought that was my preferred syntax, since you pointed out the error I made in the example (which was good, but I was a little confused at why you cared about my "worst case" example). I'm hoping we are indeed on the same page now?

And here is the question, (and this is not at all a deal-breaker for me. I think the points made above are the most important) are you also saying that only "simple" selectors can be extended, or are you saying that only specific selectors can be extended? Hopefully we are on the same page, and you do mean the latter. And if so, then do you think these two statements should be treated differently?

.shape .header {}

// and

.shape {
    .header {}
}

Personally I don't, I think that .square:extend(.shape .header) should extend either of them. This keeps it super-straightforward, easy to explain, allows you to edit your code and nest or un-nest when it makes sense, allows you to target the selectors you need to target, and it frees you from having to think "maybe I shouldn't nest this because it won't get extended anymore, or maybe I'll need to extend this in the future, so I won't nest it...". There are lots of reasons to nest or not to nest, but this should not even weigh into that decision at all.

@matthewdl what you're describing does sound undesirable, I agree. I'm not sure I follow you the whole way through your example though, could you put a code snippet of what you mean?

Soviut commented 11 years ago

Another thing to consider is that extending should be possible without any existing mixins or nested styles before it. In this way, the inline :extend makes the most sense. Nesting with extending initially feels very useful, but will ultimately lead to a lot of confusion and questions about when to nest and when not to.

The proposal to have several types of :extend, :extend-all etc. makes the most sense since we can iterate on them. Start with the basic :extend then move on to more complex applications when we actually reach the real-world limits of what it can do.

lukeapage commented 11 years ago

@DesignByOnyx good point.. the chaining example looks like its a neat way of doing things but it actually doesn't make any sense (I was struggling to define how it could make sense last night and gave up)

@matthewdl - this makes alot of sense to me

It's not that I don't get wanting to sometimes opt out of extending any nested selectors. But the solution to that seems so easy (moving the nested selectors outside of the block being extended) vs. the difficulty / unmaintainability of adding 20 extend statements (when nested selectors aren't extended), that that's why I stand by that position: that extending the entire block is far easier, and more maintanable for devs

essentially the trade off you have for putting in nested selectors is that "moving the nested selectors outside of the block being extended" only works if the extend treats .a .b { differently from .a { .b { right? and the trade off is that a small refactoring that you wouldn't think would change anything, effects the output of a block in a different file that is using an extend.

However, I guess thats how mixins work.. it matches .a { .b { and not .a .b { hrmm.

@jonschlinkert I agree with matthew that in the first instance, including nested selectors makes logical sense... but I don't want to rule out your example because I think its very useful.

In the end I go back to thinking why comprimise.. Although its more complicated having 2 extends, I think its more straight forward for the end user...

have an :extend that matches only the selector you have (exact match, e.g.

.d .a { color: white; }
.a { 
    .b { color: black;}
    background: red; 
}
.c:extend(.a) {
    color: green;
}

.d .a { color: white; }
.a, .c { background red; }
.a .b { color: black; }
.c { color: green; }

and an extend-all that is inclusive

.d .a { color: white; }
.a { 
    .b { color: black;}
    background: red; 
}
.c:extend-all(.a) {
    color: green;
}

.d .a,
.d .c { color: white; }
.a, .c { background red; }
.a .b,
.c .b { color: black; }
.c { color: green; }

It forces people to think "do I really want to extend all" which helps you not do something bad or multiply out the selectors.

@matthewdl regarding your previous comment

-1 to :extend-all. Let's not repeat @import and @import-once, where we end up with multiple syntax additions because we can't agree on behavior of :extend. I'd much rather see us arrive at consensus

And here I think the consensus went that we should have made @import work like @import-once and not added the option. But in that case the option was added (by @cloudhead I think - not to point blame :p)? because he was scared of changing the existing behaviour and actually no-one seemed to then want the @import-multiple... besides in 1.4.0 we are changing the default behaviour of @import to once and I think we can probably phase out the -once format. On that note I am still waiting on a reply to a different bug on how someone should specify options for imports inline (nudge).

I don't think that one attempt to add options to a statement that turned out we didn't need it should put us off if its the solution that makes the most sense.

Soviut commented 11 years ago

Agreed, the issue wasn't with multiple imports, it was how @import behaved by default. In that case, the default behaviour was backwards, however, in the case of :extend and :extend-all the behaviour would be far more correct and far more predictable. It also guarantees we won't end up with :extend-exact-type fixes in the future.