Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
lukeapage opened this issue Jan 31, 2013 · 158 comments
Closed

Comments

@lukeapage
Copy link
Member

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
Copy link

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
Copy link
Member

@DesignByOnyx Updated.

@DesignByOnyx
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link
Contributor

@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
Copy link

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 */
}
  1. 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.
  2. Unlink SASS, "more specific" instances of the selector should be left alone (eg .header .selector example above).
  3. 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
Copy link
Member

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
Copy link
Contributor

@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
Copy link

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
Copy link
Contributor

@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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Contributor

@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
Copy link
Member Author

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
Copy link
Contributor

@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
Copy link

@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
Copy link
Member Author

:extend

and

:extend-all

?

@matthew-dean
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member Author

@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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link

Soviut commented Feb 9, 2013

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
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Member

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

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.

@DesignByOnyx
Copy link

@lukeapage - you can still do "any shallow" without adding any new keywords:

.button {
    prop1:prop1;

    .a { prop2:prop2; }
}
#header {
    .button { 
         prop3:prop3; 

        .a { prop4:prop4; }
    }
}
.test:extend(.button shallow any) { } 
.button,
.test { prop1:prop1; }

.button .a { prop2:prop2; }

#header .button,
#header .test { prop3:prop3; }

#header .button .a { prop4:prop4; }

@custa1200
Copy link

Wow...

Sent from my iPhone

On 05/03/2013, at 5:34 PM, Ryan Wheale notifications@github.com wrote:

I really don't think you understand me. Let me address your last comment where you said:

What if I DON'T want to extend any of these with :extend(.button deep)?

.header .button { }
#main .button.button-a { }
.single-use .widget .button a:hover { }
REPLY: Nothing would happen... and I haven't made that claim... which is why I know you clearly don't understand what I am trying to say. In the above code, .button is in the context of other elements (header, main, etc). I think we all agree that "exact" should not match any of the above, and I would like to move past this example. You MUST use the "any" modifier in order to extend .button in the above example. Agreed? Moving on to the issue at hand...

Let me see if I can word this in a non-technical way. I am not trying to sound condescending when I say this: Focus ONLY on styles in which .button is the left-most part of the style declaration:

.button {
.a { prop1:prop1; }
.b { prop2:prop2; }

#header & { prop4:prop4; }

}
.button .c { prop3:prop3; }
#footer .button { prop5:prop5; }
The above gets compiled to the following CSS:

.button .a { prop1:prop1; }
.button .b { prop2:prop2; }
.button .c { prop3:prop3; }
#header .button { prop4:prop4; }
#footer .button { prop5:prop5; }
Please please read slowly here. Notice the declarations for props 1-3. See how .button is the left-most part of the selector. This means that every piece of HTML written like is going to get props 1-3, no matter if it's in the header, footer, table, div.... you get my point?

Now let's extend the button onto another element:

input[type=submit]:extend(.button deep) { }
Every instance of should get props 1-3. It doesn't matter whether it's is in the header, footer, main... it should get the same exact styles as the .button sitting right next to it in the DOM. It doesn't matter that "prop3" was written as .button .c { prop3:prop3; } because it's the same as if I had written .button { .c { prop3:prop3; } }. Again, in layman's terms the .button is the left-most* part of the selector. The fact that Developer A writes his style nested and Developer B doesn't is merely a matter of preference.

* Now let's say that the submit button should get ANY .button style:*

input[type=submit]:extend(.button deep any) { }
As we have covered ad nauseum... If I supply the "deep any" modifier (aka. "all"), then it should also get props 4-5. The reason for this is because .button is in a more specific context. In layman's terms, .button is no longer the left-most part of the selector and therefore is not an "exact" match to .button.


Reply to this email directly or view it on GitHub.

@lukeapage
Copy link
Member Author

@DesignByOnyx @MatthewDL

I was trying to implement it the way originally agreed, but I hit a snag (when you have &:hover the selector joining code moves :hover into the parent so to my code .a { &:hover { and .a:hover { look the same.. so at the moment deep/shallow isn't easy.

for the time being I had a go at something along the lines of what @DesignByOnyx is suggesting.. that boils down to two parameters, 1. match begining 2. match end

:extend(.a) // match begining and end, matches only .a { exactly
:extend(.a all) // match anywhere, matches .b.a.c {
:extend(.a deep) // match begining, so matches `.a { &:hover` and `.a:hover`
:extend(.a any) // no longer makes sense

I can refactor the way the joining works so that the original behaviour works.. or I can solidify on extend the way it works on master now.

what do you think?

I highly suggest that you start referring to the unit tests, which now have loads of :extend cases in them

@lukeapage
Copy link
Member Author

p.s. less2css.org has a cron job to pick up the latest alpha.. so I'm guessing that within a couple of hours you will be able to try out the extend functionality discussed in this post there...

@lukeapage
Copy link
Member Author

I tried explaining it to a colleague and it became clear that with the current definitions, any and deep do not make any sense. He also agreed that extend should act on its output, not its input. does the following make more sense in terms of options.. by each selector I have put the extend that will match that line

.start .a.b .end,      // :extend(all .a.b)
.a.b .end,               // :extend(from .a.b)  :extend(all .a.b)
start .a.b,               // :extend(upto .a.b)  :extend(all .a.b)
.a.b                       // :extend(.a.b) :extend(from .a.b):extend(upto .a.b) :extend(all .a.b)
{
}

@matthew-dean
Copy link
Member

He also agreed that extend should act on its output, not its input.

I think that's a fundamental difference in how some view LESS, which I mentioned probably 30,000 words ago in this post. ;-)

I think that's also contrary to how LESS blocks are interpreted.

For example, at first glance, one of these can represent the other.

.button {
  .a { }
}
.button .a { }

However, a subtle change illuminates their inequality:

.button {
  @color: blue;
  .a { @color: red; }
}
.button .a { }  // No equivalent representation i.e. I must refactor again if properties in each block use @color

They may compile the same, but they aren't treated the same. They have different variable scope. And in this case, such as in the case of mixins, our :extend syntax is a LESS construction, not a CSS one. So, it's a stretch to say that it should only pertain to the output, when nothing else (mixins, etc) behaves that way. LESS functions & mixins apply to the input, not the output.

That being said, I can see the desire to have input / output treated similarly, based on coding style, as @DesignByOnyx @lukeapage are advocating.

To me, I would love to be able to do this.

.test:extend(.button deep) {}

.button {         // extend this
  .a {  }
}
.button .a { }   // intentionally refactored / removed to not be included on the extend

As you say, whether you would personally take that approach is a matter of style / approach to LESS. (I think of it as modifying tree structures, as that's the way these blocks are represented.) But, for me, that's entirely why I've been advocating for "exact" matching, to be very granular / specific about what I'm targeting.

So, if we support multiple style preferences as you're suggesting, and perhaps we can, I would like to not see that style support removed. I just don't know how to reconcile these two views. Any ideas?

@matthew-dean
Copy link
Member

Or, if you could think of another way / syntax to target in the way I'm suggesting, I would concede the point. That is, extending one button block but not the other.

@lukeapage
Copy link
Member Author

yes, exact is great but deep is a nightmare to implement how you like.. because extend either works on the input selectors (before '&' is processed) or it matches on them afterwards. It was immensely easier to act afterwards (it makes exact shallow just work) and also allows matching the selector you are looking for, rather than whatever is in the less (most people won't care what is in the less, they just want to extend/refactor a selector). so say I have this example...

.a { .a { .b & {
:extend(.b .a deep)

we want it to match because the output selector matches what is in the extend.. but deep according to the input makes no sense what-so-ever.. (you have to start thinking about it in terms of the output in the above example, otherwise it doesn't match at all)

I tried to implement shallow/deep as you @MatthewDL want, but I've hit a sticking point, I'd have to make significant changes to the selector joining algorithm to mantain some assemblence of seperate selector paths.

As I said, I'm pushed for time, so pragmatically (not a threat.. just pragmatically!!) we either go with a revised definition or we scrap the deep/any keywords for 1.4.0 and move into a new bug. I don't want to do that, because in all likelihood, if I don't do it, it won't get done.. but maybe its actually the right thing to do, we can get some feedback on it and where people think it is lacking.

@lukeapage
Copy link
Member Author

.a { .b & .c { // shouldn't match
.a { .b & { .c { // should match
:extend(.b .a deep)

is a good example btw. of two selectors that should match/notmatch with @MatthewDL's interpretation of deep, but I think are going to be impossible to tell apart in the current code.

@DesignByOnyx
Copy link

@MatthewDL - you are correct, we need to come to some sort of consensus. Let's take a simple example we have already been using:

.button {
    .a { prop1:prop1; }
    .b { prop2:prop2; }
}
.button .c { prop3:prop3; }
test:extend(.button deep);

So we agree that your way would not match .button .c and my way does. I think we both understand why we each hold the opinion that we do. Hand shake... moving forward towards reconciliation.

For the sake of conversation, let's consider all selectors where .button is the left-most part of the selector as the "base class" for all things .button (remember that every <element class="button" /> will get these base styles). By my thinking, if something deep extends .button, all "base class" properties and child properties should get extended (I think I have made this clear, don't mean to beat the point).

In the example above, if "prop3" should no longer be part of the base class, then I need to do a multi-shallow-extend sort of like @jonschlinkert argued at the beginning of this thread:

test:extend(.button, button .a, .button .b) { }

... OR give it a new "base class": #main .button .c

.button {
    .a { prop1:prop1; }
    .b { prop2:prop2; }
}
#main .button .c { prop3:prop3; }
test:extend(.button deep);


@lukeapage - you provided the following example:

.a { .a { .b & {
:extend(.b .a deep)

You are correct that .b .a would match. The "deep" modifier would only be useful if there were actually some "deeper" styles to extend:

.a { .a { .b & { .c {
:extend(.b .a deep)

Now let me address your next example:

.a { .b & .c { // shouldn't match
.a { .b & { .c { // should match
:extend(.b .a deep)

I disagree and think BOTH should match... again because the fact that .c is nested in one but not the other is merely a preference. In both cases, .b .a is the left-most part of the selector... the "base class" if you will.

Here is how I view the "any/exact" modifiers in lay terms:

  • :extend(.b .a exact) - find all selectors where .b .a is the left-most part of the final selector
  • :extend(.b .a any) - find all selectors where .b .a is ANYwhere in the final selector (beginning, middle, or end).

@lukeapage
Copy link
Member Author

@DesignByOnyx I have edited my last example post to make it clearer the example is not what I think should happen but instead two selectors that the current code can't differentiate.. I agree with you, that both should match.

"your view" of "any/exact" is extremely confusing to the whole picture. The original "approved" idea was two properties, shallow/deep and any/exact. exact means matches entire selector only and nothing below it - something I think is a great innovation in our syntax... we do not want to redefine it to mean the "left-most part".

We are talking about the definition of shallow/deep which are sub options. When considering how extend works on the output selectors (e.g. how @DesignByOnyx wants it to work) it essentially boils down to whether you allow parts before or after - those two booleans give you the 4 names.. this was my reason for suggesting that, assuming we were to go with what you want, we must rename the options because at the moment they are confusing. Perhaps this is supported by the fact you confused them above?

@MatthewDL I forgot to also mention that your variable example works for variables, but not for mixins

.a { .b {
    background: red;
  }}
 .a .b {
    background: green;
 }
.c {
  .a.b; // matches both
}

also I kind of think that doing this

.button .a { }   // intentionally refactored / removed to not be included on the extend

is something I would recommend people not to do - how come I want to extend button and all classes related later classes but the target decides it doesn't want to be extended when used to select a child link? Shouldn't it be the extender that decides what it wants to target (with the shallow/deep options). By treating the selectors differently we are putting in this extra bit of weird behaviour over how selectors are matched and putting the control over that with whom-ever writes the source selector

I am worried this debate will go on forever (there seems to be a large amount of content generated from misunderstanding) and am still fond of only supporting no options (exact) and all, using @MatthewDL definition that everyone agreed with way back on the thread.. we can always see what feedback we get from that and leave this open to a later version.

@DesignByOnyx
Copy link

Man I don't understand how we got so off base. You make the following comment:

exact means matches entire selector only and nothing below it.

Correct.

THEN, if someone passes "deep", it will match items below it, right? By original definition, "deep" should extend all nested selectors/properties. I consider .a, .b, and .c to be "nested" below the button here... despite the syntax... and this point is really important to me:

.button {
    .a { prop1:prop1; }
    .b { prop2:prop2; }
}
.button .c { prop3:prop3; }

Now, I never... ever... ever took "deep" OR "exact" to mean that it would match .button in the following context, and I cannot understand how you can think that it should or that it's even useful:

#header {
    .button {
        .a { prop1:prop1; }
        .b { prop2:prop2; }
    }
}

If you wanted to match .button in the above example, you should have to do one of two things (note, I am leaving out "deep" because I am just trying to match .button right now):

  • :extend(#header .button) { } - This is very explicit... no confusion
  • extend(.button any) { } - This is very willy nilly "extend everything", but useful is some situations

Please think about the DOM:

<header>
    <span class="button">Button1</span>
</header>
<div role="main">
    <span class="button">Button2</span>
    <div class="call-to-action cta-1">
        <a href="#">Click Me</a>
    </div> 
    <div class="call-to-action cta-2">
        <a href="#">Click Me</a>
    </div>
</div>

And this LESS:

.button {
    .a { prop1:prop1; }
    .b { prop2:prop2; }
}
.button .c { prop3:prop3; }
#header {
    .button {
        .a { prop4:prop4; }
    }
}

/* Make all call-to-action links look like buttons */
.call-to-action a:extend(.button deep) {  /* Props 1-3 */ }

/* Make this link look just like the header button even though it's not in the header */
.cta-2 a:extend(#header .button deep) {  /* Prop4 */ }

/* Make all submit buttons get the same styles as .button */
input[type="submit"]:extend(.button deep any) { /* Props 1-4 */ }

If you still don't agree, then I am done with this thread and this whole discussion was pointless for me. This is my last comment on this issue. Please re-read my last several comments if you still need clarification on how I see it.

@ricardobeat
Copy link
Contributor

Pardon me, but what is going on here? Why can't we just leave this pseudo-selector idea and all the black magic aside, do only exact selector matches (like stylus) and implement the obvious:

.button { ... }

.action {
    extend(.button);
}

// or this, a nice complement to standard mixin syntax
.action {
    .button+;
}

I haven't read the 396 posts in this issue, so I might be just pissing all over the work done so far, but I feel like just leaving the building. It's been a couple years since this surfaced as a needed feature.

@jonschlinkert
Copy link
Contributor

@ricardobeat noted, and we agree.

This conversation has gone on this long because the core team cares so much about everyone's expectations. If you read through the discussions you would see that the discussion has gone this long because others disagreed with your point (even suggesting we do "exact match" like Stylus' implementation was made. by me actually).

Why can't we just leave this pseudo-selector idea and all the black magic aside

That boat has sailed. It was discussed at length by a number of people including cloudhead. More information can be found on the original request for extend, submitted just over a year ago: #509.

I feel like just leaving the building. It's been a couple years since this surfaced as a needed feature.

I hear you about the frustration, I felt like leaving the building too before @MatthewDL and @lukeapage graciously stepped up and turned the project around, gratis. Remember that we're all open source contributors giving to this is what little free time we have.

Stay passionate about seeing progress with LESS, but be patient. We're going to get this resolved.

@dmi3y
Copy link

dmi3y commented Mar 6, 2013

I was thinking around it, sorry still in reading all the posts, so would probably miss something, as my point possible way is extending vocabulary, like Luke suggested with from/upto, let's say something like

.button {
    .a { prop1:prop1; }
    .b { prop2:prop2; }
}
.button .c { prop3:prop3; }

.test:extend(.button before .c) // before/after sounds familiar, by nature deep and exact, though it might be fun to have something shallow here;)

I also see reason in scope behavior suggested by Matthew, though this approach looks less attractive in situation when you need make quick patch to someone's spagetti code and you are not entirely sure about what's the code structure is, if it is consistent or not, and so you will have to go into deeper refactoring.

And some thoughts about any keyword as I got it, sorry for repeating it.

.button {
    .a { prop1:prop1; }
    .b { prop2:prop2; }
}
.button .c { prop3:prop3; }

#less:extend(.c any) { ... }

makes class substitution?

...
.button .c,
.button #less { ... }

or just replacement of the whole selector?

 ....
.button .c,
#less { ... }    

I think it is first, although could you think over situations when the latter could be applicable?

@dmi3y
Copy link

dmi3y commented Mar 6, 2013

Why can't we just leave this pseudo-selector idea and all the black magic aside

erh, suppose, because it is not so funny;) (gone to prepare css/html/javascript elecsir:D)

PS: I am sorry for off top, if you feel it is insulting just let me know and I will remove it.

@ricardobeat
Copy link
Contributor

@jonschlinkert yes, I followed that issue. at some point it was simply arbitrarily decided that the syntax was done, just like it had been changed to ++ a few months earlier with a pair of opinions. I don't see how a pre-processor directive belongs amongst a CSS selector. It has no parallel to jQuery filters or any pseudo-selectors as hinted in that discussion.

While the original SASS-like +.button; syntax was certainly confusing, putting the + at the end is nice: .button; copies/beams down properties, .button+; adds/beams up the selector, and could be just syntax sugar for a extend(.button) method. It's current a syntax error, so not ambiguous at all.

Anyway, I know I'm late to the party. As long as &:extend is supported and the any, deep insanity is ignored I think everyone will be happy :)

@Merg1255
Copy link

Merg1255 commented Mar 6, 2013

since this is the only remaining issue for 1.4, do we have any news on where about it will be released?..

@lukeapage
Copy link
Member Author

We (me, Jon, Matthew) have decided to go with exact (no options) and all for now and revisit this after release and outside reactions.

You can try this out today on less2css.org (select drop down then alpha) and I'm planning on releasing a beta today if I finish chaining.

@jonschlinkert
Copy link
Contributor

👍 nice job, thank you @lukeapage.

@Merg1255
Copy link

Merg1255 commented Mar 7, 2013

thank you @lukeapage ! :)

@lukeapage
Copy link
Member Author

Yes it did.
http://lesscss.org/features/
Read the extend section

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

No branches or pull requests

10 participants