Building mixins should combine duplicate nested selectors

[mlms13]

I often find myself avoiding mixins because I know that the compiled CSS will have duplicate selectors, which makes for a larger file. For example, a reduced test case:

.mixin () {
    a {
        color: red;
    }
}
div {
    .mixin();
    a {
        text-decoration: none;
    }
}

When this is compiled, the resulting output is:

div a {
    color: red;
}
div a {
    text-decoration: none;
}

In a perfect world, selectors inside a mixin and selectors inside the scope in which the mixin was executed would be combined when they are identical. Then, the resulting output would be the much cleaner:

div a {
    color: red;
    text-decoration: none;
}

[euskadi31]

+1

[krnlde]

+1

[scottgit]

If implemented, this would need to not be automatic ( except, perhaps, in the case of the two directly following one another in the code, as the minimal example above does), and probably something that needs to be switched for each instantiation of the mixin (rather than a global setting, though a global setting might control the default, with the individual switch either turning on or off the setting--whatever is opposite the global setting). This is because of various possible html configurations and cascade concerns that are impossible to know "automatically," and thus should be individually thought through by the designer/coder.

In short, this can cause issues:

Assume this html:

<a class="test1 test2>Some link</a>

Assume this case of mixin (also "simplified"):

.mixin () {
    .test1 {
        color: red;
    }
}
div {
    .test1 {
        text-decoration: none;
    }

    .test2 {
        color: blue;
        text-decoration: underline;
    }

   .mixin();
}

If mixin() merges above .test2 into .test1 then the color will come out blue not red in the final CSS. If the .test1 merges below .test2 with the .mixin() code, then text-decoration becomes none not underline. So in either case, the merging of the .test1 with the mixin() code does not produce the same styling as the code above, which would yield a color: red and text-decoration: underline by the cascade.

For further discussion on the issues of any automated form of this, see this Stack Overflow answer to a question about this.

[jonschlinkert]

@scottgit IMO, the example you pointed to is a better example of how to write brittle code.

In any case, this is easily resolved by making automatic merging an option, and make it false by default.

[scottgit]

@jonschlinkert Thanks for you further thoughts. It may be a bit brittle (and looks more so in the minimal test case), in that it relies on equal specificity and the cascade. But if you consider that this scenario is actually quite common in a situation where a framework library is providing the original definitions of .test1 and .test2 and a site developer comes along and redefines .test1 via the mixin in their own code later, you could easily end up with exactly this scenario, perhaps without even really "thinking" about it.

The "rule replacement" feature you linked to would give more individual control, but it would still need to be determined if the replacement occurs in the css cascade at the point of the original or at the point of the replacement code, because with output css, the cascade can and does matter.

This is why even if automatic merging is set to false by default, if one wants to merge at one point or another, it should be an explicit statement and IMO ideally allow the option for the merge to happen either at the original point or the new point in the cascade. And if the auto merging is set instead to true, that one can override the merge in a case where it might not be desired because one is wanting to use the cascade. This would allow the most flexibility in controlling the design of the final css output, including cascade order.

[jonschlinkert]

@scottgit thanks for sharing further thoughts. That makes a lot of sense, I'm convinced.

However, I suggest that you create another issue to make a feature request for to allow controlling at the ruleset or selector level, since technically that isn't mutually exclusive to this feature.

In other words, if this is implemented exactly as requested in this issue (with the option to globally merge, or not merge), then a user such as yourself might choose to always keep the feature turned off. But there are other users who would keep it turned on, since there are plenty of use cases where it wouldn't be necessary to specify at the ruleset level whether or not properties should be merged. So let's keep this feature simple, and ideally have another feature request that augments this with more granular control.

[dotnetwise]

This is still not implemented.

[smmoosavi]

+1

[schollD]

+1

[ghost]

+1

[matthew-dean]

Btw, since there was some debate in this thread: at the very least, adjacent duplicate selectors can safely be merged, which is the example given at the top of the discussion. Non-adjacent duplicate selectors cannot safely be merged by default (but could via an option).

[krnlde]

That's the whole point.

[seven-phases-max]

Well, by now adjacent selectors are combined if you compile with --clean-css --clean-option=--advanced (or just use clean-css with default options in your build toolchain). So I wonder if anyone would want to put his time and efforts to implement something that is already possible by other methods. Either way ReadyForImplementation label means "PRs are welcome" so...