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

Extend of extended selector #1641

Open
SomMeri opened this issue Nov 6, 2013 · 6 comments
Open

Extend of extended selector #1641

SomMeri opened this issue Nov 6, 2013 · 6 comments

Comments

@SomMeri
Copy link
Member

SomMeri commented Nov 6, 2013

Extend all does not always find matches in selector generated by another extend. Such match can but does not have to be found, depending on circumstances:

  • Two extend keywords are not going to be applied together into the same selector.
  • Extend will be applied into another extending selector and the result will be used as third extend. They create new extending selector on the fly and that one is applied to the rest of the sheet.

The combination of these rules make it hard to predict what will be generated. It is also very hard to describe for documentation :) (or for newbie). I think that two extends matching the same selector should either act independently or together at the same selector, but should not create new extending selectors on the fly.

Sorry if it sounds confusing. I tried to isolate cases that work and does not work and describe them all below.

1. Two Extend All of The Same Selector

If two extend all match the same selector, less.js will not combine them. However, both of them will extend original selector:

a:first b:second {
    property: value;
}
exSelector:pseudo:extend(a:first all) {}
independentExtend:extend(b:second all) {}

compiles into:

a:first b:second,
exSelector:pseudo b:second,
a:first independentExtend { // there is no exSelector:pseudo independentExtend
  property: value;
}

I expected one of following:

//option 1
a:first b:second,
exSelector:pseudo b:second,
a:first independentExtend,
exSelector:pseudo independentExtend { 
  property: value;
}

//option 2
a:first b:second,
exSelector:pseudo independentExtend {
  property: value;
}

2. Extend All Extending Selector

If extend all matches something inside another extend all, then it is going to be extended too:

a:first b:second {
    property: value;
}
exSelector:pseudo:extend(a:first all) {}
partialMatch:extend(exSelector all) {} // extends previous extend

compiles into:

a:first b:second,
exSelector:pseudo b:second,
partialMatch:pseudo b:second {
  property: value;
}

Less.js created partialMatch:pseudo:extend(a:first all) {} on the fly and applied it to the ruleset.

3. Extend All of Extended Selector

If extend all of matches extended selector, less.js will not find it: (EDIT: fixed the example, the original version had mistake in it)

a:first b:second {
    property: value;
}
exSelector:extend(a:first all) {}
longExtendExtend:extend(exSelector b:second all) {} //extend extended selector

compiles into:

a:first b:second,
exSelector b:second { //longExtendExtend was not used
  property: value;
}

I expected one of the following:

// option 1
a:first b:second,
longExtendExtend  { 
  property: value;
}

// option 2
a:first b:second,
exSelector:pseudo b:second,
longExtendExtend  { 
  property: value;
}

All In One - Full Case

The main question is, what should be generated by following less:

a:first exSelector { // original ruleset
    property: value;
}
exSelector:extend(a:first all) {} // extends original ruleset
chained:extend(exSelector all) {} // extends previous extend and original ruleset

As it is now, less.js produces this:

a:first exSelector,
exSelector exSelector, // applied exSelector:extend(a:first all)
a:first chained, // applied chained:extend(exSelector all)
chained exSelector { // sort of applied both - I expected chained:pseudo chained
  property: value;
}

I expected it to contain following selector: chained:pseudo chained. I did not expected the chained exSelector to be there, but it can be explained, so it may be ok.

@seven-phases-max
Copy link
Member

here was some false alarm talk about (3)

@SomMeri
Copy link
Member Author

SomMeri commented Nov 6, 2013

@seven-phases-max If extend all would treat selector elements independently, then it would be impossible to match combinators. They are compared now, so current version behaves as (B):

a:first b:second  { property: value;}
a:first + b:second  { property: value;}

descendant:extend(a:first b:second) {}
asibling:extend(a:first + b:second) {}

descendantAll:extend(a:first b:second all) {}
asiblingAll:extend(a:first + b:second all) {}

compiles into:

a:first b:second,
descendant,
descendantAll {
  property: value;
}
a:first + b:second,
asibling,
asiblingAll {
  property: value;
}

The combinator must be in the middle, leading and trailing combinators do not work. I think that this one is separate issue #1642 .

@seven-phases-max
Copy link
Member

@SomMeri, Yes, (B) is what I suspected. But then I also expect (3)'s longExtendExtend:extend(exSelector b:second all) to never match exSelector:pseudo b:second due to missing :pseudo. What do you think?

@SomMeri
Copy link
Member Author

SomMeri commented Nov 7, 2013

@seven-phases-max I say, good catch. I was simplifying example and forgot about that pseudo, sorry.

The example should look like this:

a:first b:second {
    property: value;
}
exSelector:extend(a:first all) {}
longExtendExtend:extend(exSelector b:second all) {} //extend extended selector

and compiles into:

a:first b:second,
exSelector b:second {
  property: value;
}

So, longExtendExtend:extend(exSelector b:second all) does not match generated exSelector b:second.

I will edit the above example, so others do not waste time with the same mistake.

@seven-phases-max
Copy link
Member

Ah, OK - then I'll wash out my first post since it simply repeats (3).

@lukeapage
Copy link
Member

lukeapage commented May 27, 2014

https://github.com/less/less.js/blob/master/lib/less/visitors/extend-visitor.js

Looks like we need to rip out the chaining concept there and re-implement it so that every new selector that is created has all of the extends processed on it - this would allow you to extend a selector created via another extend.

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