support media queries in extend chaining. Also tidied up. Fixes less#1213

Author: lukeapage

lib/less/extend-visitor.js

@@ -45,6 +45,7 @@
                     extend = extendList[j];
                     extend.findSelfSelectors(selectorPath);
                     extend.ruleset = rulesetNode;
+                    if (j === 0) { extend.firstExtendOnThisSelectorPath = true; }
                     this.allExtendsStack[this.allExtendsStack.length-1].push(extend);
                 }
             }
@@ -86,37 +87,75 @@
             return this._visitor.visit(root);
         },
         doExtendChaining: function (extendsList, extendsListTarget, iterationCount) {
-            var extendIndex, targetExtendIndex, matches, extendsToAdd = [], newSelector, extendVisitor = this, selectorPath;
+            //
+            // chaining is different from normal extension.. if we extend an extend then we are not just copying, altering and pasting
+            // the selector we would do normally, but we are also adding an extend with the same target selector
+            // this means this new extend can then go and alter other extends
+            //
+            // this method deals with all the chaining work - without it, extend is flat and doesn't work on other extend selectors
+            // this is also the most expensive.. and a match on one selector can cause an extension of a selector we had already processed if
+            // we look at each selector at a time, as is done in visitRuleset
+
+            var extendIndex, targetExtendIndex, matches, extendsToAdd = [], newSelector, extendVisitor = this, selectorPath, extend, targetExtend;
 
             iterationCount = iterationCount || 0;
 
+            //loop through comparing every extend with every target extend.
+            // a target extend is the one on the ruleset we are looking at copy/edit/pasting in place
+            // e.g.  .a:extend(.b) {}  and .b:extend(.c) {} then the first extend extends the second one
+            // and the second is the target.
+            // the seperation into two lists allows us to process a subset of chains with a bigger set, as is the
+            // case when processing media queries
             for(extendIndex = 0; extendIndex < extendsList.length; extendIndex++){
                 for(targetExtendIndex = 0; targetExtendIndex < extendsListTarget.length; targetExtendIndex++){
 
-                    var extend = extendsList[extendIndex];
-                    var targetExtend = extendsListTarget[targetExtendIndex];
+                    extend = extendsList[extendIndex];
+                    targetExtend = extendsListTarget[targetExtendIndex];
+
+                    // look for circular references
                     if (this.inInheritanceChain(targetExtend, extend)) { continue; }
 
+                    // find a match in the target extends self selector (the bit before :extend)
                     selectorPath = [targetExtend.selfSelectors[0]];
                     matches = extendVisitor.findMatch(extend, selectorPath);
 
                     if (matches.length) {
 
+                        // we found a match, so for each self selector..
                         extend.selfSelectors.forEach(function(selfSelector) {
+
+                            // process the extend as usual
                             newSelector = extendVisitor.extendSelector(matches, selectorPath, selfSelector);
+
+                            // but now we create a new extend from it
                             newExtend = new(tree.Extend)(targetExtend.selector, targetExtend.option, 0);
                             newExtend.selfSelectors = newSelector;
+
+                            // add the extend onto the list of extends for that selector
                             newSelector[newSelector.length-1].extendList = [newExtend];
+
+                            // record that we need to add it.
                             extendsToAdd.push(newExtend);
                             newExtend.ruleset = targetExtend.ruleset;
+
+                            //remember its parents for circular references
                             newExtend.parents = [targetExtend, extend];
-                            targetExtend.ruleset.paths.push(newSelector);
+
+                            // only process the selector once.. if we have :extend(.a,.b) then multiple
+                            // extends will look at the same selector path, so when extending
+                            // we know that any others will be duplicates in terms of what is added to the css
+                            if (targetExtend.firstExtendOnThisSelectorPath) {
+                                newExtend.firstExtendOnThisSelectorPath = true;
+                                targetExtend.ruleset.paths.push(newSelector);
+                            }
                         });
                     }
                 }
             }
 
             if (extendsToAdd.length) {
+                // try to detect circular references to stop a stack overflow.
+                // may no longer be needed.
                 this.extendChainCount++;
                 if (iterationCount > 100) {
                     var selectorOne = "{unable to calculate}";
@@ -129,6 +168,8 @@
                     catch(e) {}
                     throw {message: "extend circular reference detected. One of the circular extends is currently:"+selectorOne+":extend(" + selectorTwo+")"};
                 }
+
+                // now process the new extends on the existing rules so that we can handle a extending b extending c ectending d extending e...
                 return extendsToAdd.concat(extendVisitor.doExtendChaining(extendsToAdd, extendsListTarget, iterationCount+1));
             } else {
                 return extendsToAdd;
@@ -308,13 +349,17 @@
         visitRulesetOut: function (rulesetNode) {
         },
         visitMedia: function (mediaNode, visitArgs) {
-            this.allExtendsStack.push(mediaNode.allExtends.concat(this.allExtendsStack[this.allExtendsStack.length-1]));
+            var newAllExtends = mediaNode.allExtends.concat(this.allExtendsStack[this.allExtendsStack.length-1]);
+            newAllExtends = newAllExtends.concat(this.doExtendChaining(newAllExtends, mediaNode.allExtends));
+            this.allExtendsStack.push(newAllExtends);
         },
         visitMediaOut: function (mediaNode) {
             this.allExtendsStack.length = this.allExtendsStack.length - 1;
         },
         visitDirective: function (directiveNode, visitArgs) {
-            this.allExtendsStack.push(directiveNode.allExtends.concat(this.allExtendsStack[this.allExtendsStack.length-1]));
+            var newAllExtends = directiveNode.allExtends.concat(this.allExtendsStack[this.allExtendsStack.length-1]);
+            newAllExtends = newAllExtends.concat(this.doExtendChaining(newAllExtends, directiveNode.allExtends));
+            this.allExtendsStack.push(newAllExtends);
         },
         visitDirectiveOut: function (directiveNode) {
             this.allExtendsStack.length = this.allExtendsStack.length - 1;

test/css/extend-chaining.css

@@ -51,3 +51,22 @@
 .y {
   color: z;
 }
+@media tv {
+  .ma,
+  .mb,
+  .mc {
+    color: black;
+  }
+  .md,
+  .ma,
+  .mb,
+  .mc {
+    color: white;
+  }
+}
+@media tv and plasma {
+  .me,
+  .mf {
+    background: red;
+  }
+}

test/less/extend-chaining.less

@@ -57,4 +57,23 @@
 }
 .z:extend(.y) {
   color: z;
-}
+}
+
+// media queries - dont extend outside, do extend inside
+
+@media tv {
+  .ma:extend(.a,.b,.c,.d,.e,.f,.g,.h,.i,.j,.k,.l,.m,.n,.o,.p,.q,.r,.s,.t,.u,.v,.w,.x,.y,.z,.md) {
+    color: black;
+  }
+  .md {
+    color: white;
+  }
+  @media plasma {
+    .me, .mf {
+      &:extend(.mb,.md);
+      background: red;
+    }
+  }
+}
+.mb:extend(.ma) {};
+.mc:extend(.mb) {};