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 proposed feature set for :extend #2101

Open
matthew-dean opened this issue Jul 10, 2014 · 25 comments
Open

The proposed feature set for :extend #2101

matthew-dean opened this issue Jul 10, 2014 · 25 comments

Comments

@matthew-dean
Copy link
Member

On issue #1155, there was community consensus on the usage of extend in a way that offered flexibility for different use cases but kept things simple through the use of keyword flags. I've pulled out the relevant section/syntax that was agreed upon:

:extend( selector [exact|any] [shallow|deep] [all])

  • exact [default] - Match complete selectors (:extend(.button exact) matches .button {} but not #main .button a - :extend(.a .b) is considered an exact match to .a { .b { } })
  • any - Match partial selector (:extend(.button any) matches .button {} and #main .button a)
  • shallow [default] - extend only root properties (.a:extend(.button shallow) will not include hover in .button { &:hover { } })
  • deep - extend all nested selectors/properties (.a:extend(.button deep) will include hover in .button { &:hover { } })
  • all - set "any" and "deep" to true. Extend all matches, partial and exact, and extend all base and nested properties

Example:

.b:extend(.a);  // because of defaults, exact=true, shallow=true
.b:extend(.a any);  // exact=false, shallow=true
.b:extend(.a deep);  //exact=true, shallow=false
.b:extend(.a all); // exact=false, shallow=false

The reasons, arguments, and use cases for these other uses of :extend are documented in issue #1155. After the discussion and consensus, the first case and last case were implemented because they were easier to implement first, but I think the remaining implementation got lost in the noise. I'd love to see the rest of :extend implemented as I think these other use cases are still valid and there's various issues that come up here that essentially relate to what's still missing. (See: less/less-docs#177).

I'd love to see the work on :extend continue! (And other great ideas, like extending mixins.)

@lukeapage
Copy link
Member

Yeh, I think extending mixins is more important. It's interesting no one has brought up other extend options (till you did)

@lukeapage
Copy link
Member

I've added needs decision because I'd like to see how much complexity it adds before we decide whether it's still needed. Anyone wanting to implement, just write out a plan and let's discuss.

@matthew-dean
Copy link
Member Author

Why does it need a decision if consensus was already made? It took a long time for us to reach this decision. Doesn't it just need implementation?

I don't disagree that extending mixins is also important, though.

@seven-phases-max
Copy link
Member

A few raw and quick thoughts:
It's just two new keywords in the list: deep and any (and if I'm not mistaken we don't need to provide explicit shallow and exact since they are there by default and need only to be overriden by others but not in opposite). I think deep is actually very important because currently in at least half of the cases where all is used they actually really mean just deep. Contrary for any I can't invent any practical example at all :)

Also as discussed in other topics a scoped (i.e. "searching for a match from the current scope upwards") keyword would also be valuable. More over personally I believe that :extend(... deep scoped) because of its mixin semantics will eventually kick out all other extend variants to a ghetto. So that only all will be used from time to time and all others (incl. "default" btw.) will pass into nothingness :)


But that's just thoughts. Eventually I would even like to implement this myself but to be honest I doubt this really would happen "soon"... So I guess this all also hardly depends on who's going to implement it. Obviously we don't need everything at once so some evolutionary advances in tiny steps should not make it too complex (e.g. try deep first and we'll see how it goes?)...

@matthew-dean
Copy link
Member Author

I think deep is actually very important because currently in at least half of the cases where all is used they actually really mean just deep.

Yes, exactly. To me, that was the more logical and intuitive default use case, and probably the only use of extend I would (personally) use / promote. "all" is excessively greedy for average use, and shallow exact (default) is unintuitively narrow (because it deviates from mixin behavior).

@lukeapage
Copy link
Member

Fine, changed.

We reached consensus on what to do, I just thought from the lack of feedback that the hunger had gone..

@matthew-dean
Copy link
Member Author

Thanks.

Maybe Github should have a voting mechanism for top features to implement. Some of my pet favorites are:

  • this
  • @import (plugin) myplugin.js
  • extending mixins

@matthew-dean matthew-dean changed the title Let's finish implementation of :extend The proposed feature set for :extend Feb 3, 2015
@matthew-dean
Copy link
Member Author

What do you think about supporting an (optional) exclamation point on flags for clarity / separation from element selectors?

.myarticle {
  &:extend(section aside !all); 
}

I know our keywords don't conflict, but it might be intuitive for some because of !important.

@seven-phases-max
Copy link
Member

Btw., since the PR is coming (@bassjobsen 👍), I'd want to stress my concerns about any once again: are there any use-cases for it? (of course it can't harm but it's Less tradition to not add something just "because it can be added").

@seven-phases-max
Copy link
Member

@matthew-dean

exclamation point, could be, though personally I more like an idea previously suggested by @lukeapage (I can't find the issue though): to play with caps, e.g.&:extend(section aside All) or even &:extend(section aside ALL);. Ehrr, wrong, see below.

@matthew-dean
Copy link
Member Author

Are there really any use cases for all? I still doubt it does exactly what people mean for it to do. But, for the sake of argument, since no flag turns two switches to "false", and all turns two switches to true, the thinking was to support the two missing flags. BUT you may be right in that any is the least useful of any keyword. It's strange for someone to decide they want to extend a selector greedily in its matching, but not be greedy in its tree consumption.

Personally, if we just add deep, I'm happy (mostly... I'd love to make it default for myself at least... another use case for @plugin ?).

@seven-phases-max
Copy link
Member

Well, all can be used when you want to create just an alias for an existing class (e.g. in your HTML you just want to use .button instead of .btn everywhere). But I can't invent any for any.
Btw., curiously it turned out that there may be different interpretations of the above any description (but still neither one make too much sense).
So yes, personally I'd also stick with just deep for now. (Not forgetting scope/scoped of course but that's another story since it probably requires an implementation that quite orthogonal to all those "flat" matches above).

@matthew-dean
Copy link
Member Author

Per the pull request discussion, and reviewing that code, I can actually see where any could be quite valuable: where you essentially want to piggy-back and group the selectors from one node of a tree. So it allows you to structure a tree logically (for the author), and then join other selectors which have a like appearance as one node, but don't have the same appearance or need the same child inheritance.

Because those two similar selectors can't yet extend a detached ruleset or a mixin, there really is no clean way to refactor and keep a logical structure (and keep your rules minimized). So there's a particular pattern that any would provide that isn't provided by any other method in Less. Being able to "extract" and extend a node's rules but not it's descendants offers some interesting possibilities.

My guess is simply providing the tool will surface other use cases / patterns like that that aren't immediately obvious.

EDIT: It should be noted though that the default use of extend without a keyword does much of this already. So I guess the only added pattern to this is greedy selector matching. I just haven't really used extend much in my own styles so I haven't really toyed with these other patterns.

@seven-phases-max
Copy link
Member

to correct myself:

&:extend(section aside All)

My mistake, it's actually exactly the opposite, extend keywords should remain case-sensitive and lowercase. It's selectors themselves that are assumed to become case-insensitive there (because by now they are not, ref: #2219), e.g.:

extend(body All) // extends body all
extend(body all) // extends body with all flag

@seven-phases-max
Copy link
Member

@matthew-dean Could you please provide a minimal example that shows the difference between all and any? Probably if I understand how it is supposed to work (I don't after all of above stuff :) I could modify the PR to meet the requirements (if changes are not too deep).

@matthew-dean
Copy link
Member Author

There are some examples in #2506 but I'll do my best to explain from my perspective in some more detail.

So, the easiest way to understand or test any and deep against all is that all is a conglomerate of those features.

Therefore, there are some examples in which any and all should produce the exact same output—namely, when the selector(s) it refers to do not contain nested blocks—just as there are some examples in which deep and all should produce the exact same output—namely, when there is an "exact" or complete match of the selector.

So, in #2506, the way I generated the output is simply by taking two cases of all and generating the output in the current Less engine. Given:

.a button .q {
    color: red;
}

These two statements should produce identical output:

.b:extend(button any) {
    background: black;
}
.b:extend(button all) {
    background: black;
}

Because "all" is basically:

.b:extend(button any deep) {
    background: black;
}

deep doesn't change anything here because there is no nesting.

Where you'll see differing behavior for any and all is in nested trees, because you're implicitly calling two sets of keywords:
any = any shallow (since shallow is default)
all = any deep

So, that means that any will look like all at that particular node, but not in any child nodes. Therefore, given:

.a {
  .b {
    color: red;
    .c {
      line-height: 1;
    }
  }
}

Here is what I would expect from each output:

// using all or ALL or !all
.d:extend(.b all) { }

// output today - testable with current parser
.a .b,
.a .d {
  color: red;
}
.a .b .c,
.a .d .c {
  line-height: 1;
}
// using any or ANY or !any
.d:extend(.b any) { }

// expected output
.a .b,
.a .d {
  color: red;
}
.a .b .c {
  line-height: 1;
}

So, it's pretty straightforward. any extends the rules at that particular node and then stops. For a slightly more extended example:

.d:extend(.b any) { background: blue; }

// output
.a .b,
.a .d {
  color: red;
}
.a .b .c {
  line-height: 1;
}
.d {
  background: blue;
}

Or, to demonstrate "joining" to the center node:

.d:extend(.a .b any) { background: blue; }

// output
.a .b,
.d {
  color: red;
}
.a .b .c {
  line-height: 1;
}
.d {
  background: blue;
}

AND, if you were paying attention, you might have caught that the above is the exact same output as writing:

.d:extend(.a .b) { background: blue; }

The reason is that .a .b is an exact match, so in this case the keywords
exact shallow = any shallow
because the selector passed to extend was an exact (computed) selector.

So, it actually should be really testable, because it should behave like all on that matching node, and then look like it "matched nothing" (or, essentially, don't extend anything) on any nested block after that (and therefore behaves similar to default behavior).

I think that should be pretty comprehensive. The important thing to remember is just that all of this behavior is already there in :extend; it was just never separated into its two components. So it's fairly easy to compare and intuit the output, because all along, you've seen pieces of these behaviors with and without the all keyword. In some sense, these two keywords are not even an "addition" to extend behavior so much as explicit separation of behavior.

Does this help clarify?

@seven-phases-max
Copy link
Member

Ok, now I guess I see what is the trick. And I'm afraid in this state we'll have to postpone any because it requires another extend algorithm/separate-pass (somewhat similar to what scope would). Currently extend works on rendered selectors (wel, sort of it's actually a bit more complicated than this) not on "nodes" and in this way these two examples above are not possible to be both valid for any.
I.e. from current extend algo point of view this:

.a {
  .b {
    .c {
      line-height: 1;
    }
  }
}

is identical to this:

.a .b .c {line-height: 1}

So

  • if in the first example .b:extend(button any) applies to .a button .q,
  • in the second example .d:extend(.b any) can't stop at .b because there's no separate .b anymore but only .a .b and .a .b. c selectors (and .a .b .c is the same pattern as .a button .q).

Does this help?

P.S. Just to clarify, in this PR the extend any stops at b for the second example (so the result is what you expect) but it also stops at button in the first example so it does not apply to .a button .q.

@seven-phases-max
Copy link
Member

And just in case, to make sure... what result you would expect for this code:

.a {
  .b .c {
      line-height: 1;
  }
}

.d:extend(.b any) {}

?

@seven-phases-max
Copy link
Member

(Speaking of the scope algo - I did not think of it too deep yet but most likely it seems it also will work on rendered selectors but unlike current algorithm (which starts at the root) it will start at the current scope and just try to match rendered selectors of the current scope and go upper and upper until the match is found (basically just like mixin matching works, obviously because this mixin-like behaviour is what scope was invented for)).

@seven-phases-max
Copy link
Member

So... what if we just disable any in this PR and merge only the deep part? I guess deep itself is useful enough to be released on its own (sadly nobody joined our naming conventions convention but... eh... never mind 🍄 :)

@matthew-dean
Copy link
Member Author

Yes, I think see the problem. If that's the case that extend is applied after the collapse, how does the "all" algorithm do deep--. ohhhhh.... it does because it's matching a part of the selector, so post-collapse of a tree, it simply matches part of the result. D'oh!

So... what if we just disable any in this PR and merge only the deep part?

Yes, definitely "deep" is useful. Since you have a good understanding of the intent of it, what do you think about "any"'s usefulness? And the naming of both now? Since it seems less useful than deep, and it's an algorithmic change, we could postpone indefinitely.

Just curious, since everything collapses, how would deep actually work currently?

Also, what issue defines scope?

@seven-phases-max
Copy link
Member

Also, what issue defines scope?

I guess there's none. (There were some snippets here and there but nothing especially dedicated). In summary it's just:

.apple {
    .banana {
        b: b;
        .cherry {
            .durian:extend(.banana scope) {}
            // ^ matches first .banana in or above .cherry scope
            c: c;
        }
    }
}

css result:

.apple .banana,
.apple .banana .cherry .durian {
  b: b;
}
.apple .banana .cherry {
  c: c;
}

In other words it matches exactly like a mixin with the same name would (except that mixins ignore combinators but extend won't... and for complete "mixin emulation" it should be deep scope).

@seven-phases-max
Copy link
Member

how does the "all" algorithm do deep--. ohhhhh.... it does because it's matching a part of the selector

Strictly speaking the tree is not collapsed yet, so the nodes are there and it iterates through the nodes but it just joins node's selectors into arrays across and compares those resulting arrays (at least how I understand it - well, it's all there, you can see how difficult it looks to describe :). I.e. any probably can be put into the same code somehow but it needs more deep changes then this PR offers (or at least these changes go far beyond of my understandings of the code there :).

@matthew-dean
Copy link
Member Author

Wouldn't any do the same as scope in that example (except that it wouldn't be limited to that scope)? In any case, at this point, my suggestion would be to do deep only as you suggested, and hit some higher-priority features before any, like extending mixins or extending rulesets, or allowing @@var for rulesets or any of the various namespacing features on our list.

@seven-phases-max
Copy link
Member

Wouldn't any do the same as scope in that example

Curiously it will. Though for more complex examples the similarity ends (e.g. if there more bananas, only the lowest one is matched with scope, and yes, it's limited only to that particular scope chain, bananas within other rulesets/namespaces won't match).
(Just in case I updated the example above with expected CSS result and fixed its incorrect formatting, sorry, wrote it in rush).

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

3 participants