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

Class using mixin of same name generates double CSS #1437

Closed
mdo opened this issue Jul 22, 2013 · 4 comments
Closed

Class using mixin of same name generates double CSS #1437

mdo opened this issue Jul 22, 2013 · 4 comments

Comments

@mdo
Copy link

mdo commented Jul 22, 2013

In Bootstrap 3 we've made a change to avoid generating CSS from our mixins.less file by doing something like the following:

// Mixin
.clearfix() {
  ...
}

// Assign the mixin to the class of a same name
.clearfix {
  .clearfix();
}

While this compiles no problem, it results in duplicate code in the generated CSS whenever the mixin is used.

Should we be doing this? If it's indeed valid Less (which it seems to be since it's compiling), I suppose this is a bug then?

(Relevant issue from Bootstrap: twbs/bootstrap#8557)

@matthew-dean
Copy link
Member

Hey Mark!

You could do the following:

.clearfix {
  clearfix: value;
}

.box1:extend(.clearfix) {
   added: value;
}

Results in:

.clearfix,
.box1 {
  clearfix: value;
}
.box1 {
  added: value;
}

Note that this will result in the base class (.clearfix) being output. If you didn't want .clearfix to appear as a distinct CSS class, we're currently implementing two features to address that. (Note that this are not yet available.) One is extending mixins:

.clearfix() {
  clearfix: value;
}

.box1:extend(.clearfix) {
   added: value;
}

Which will result in:

.box1 {
  clearfix: value;
}
.box1 {
  added: value;
}

The other which is further in development is referencing a less file by reference.

@import (reference) mixins.less;

This will result in no CSS properties being generated until they are extended or used as a mixin. Bootstrap is actually one of the examples used for the reference feature, where people could, by default, import the entire Bootstrap library, but essentially map customized sections of the library to CSS classes they designate, as in:

@import (reference) boostrap.less;

.button-primary-because-i-like-long-class-names {
  .btn-primary;
}

// entire CSS output
.button-primary-because-i-like-long-class-names {
  // ... btn-primary properties and values
}

Try out the new LESS :extend feature. I think it would work great for minimizing CSS output for libraries like Bootstrap, especially for the use of clearfix.

Hopefully, we'll have mixin extending implemented soon, but we're still working out the syntax.

@jonschlinkert
Copy link
Contributor

@mdo here is a comparison of using mixins to do this versus extend. (I used http://less2css.org/, screen caps were faster to illustrate...).

Keep in mind that, by default, :extend only targets exact selector specified. So you will need to use all to target nested selectors as well. Like this: :extend(.clearfix all)

Using Mixins

image

Using :extend

image

Hope this helps, let us know if we can do anything else to help.

(btw, this is a good use case for extending mixins.)

@jonschlinkert
Copy link
Contributor

lol, sorry I started my answer earlier today then came back and didn't scroll to see that I basically duplicated @matthew-dean's answer. my bad

FWIW, I started using this convention (from the :extend example) with a number of non-parametric mixins, it really reduces generated CSS. There is a lot of opportunity to reduce code for grids, and other components with a lot of repetition.

@matthew-dean
Copy link
Member

@jonschlinkert You added the bit about the "all" keyword, which is important info, especially for the traditional clearfix set of properties. :-)

mdo added a commit to twbs/bootstrap that referenced this issue Dec 9, 2013
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes #8947, #8968, #8991, #9257, #9268, #9291, #9430, #9604, #9686,
#9929, #10731, #10793, #11305, #11498, #11533, #11570, #11604, #11652.

(dem issues, tho)
stempler pushed a commit to stempler/bootstrap that referenced this issue Apr 11, 2014
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes twbs#8947, twbs#8968, twbs#8991, twbs#9257, twbs#9268, twbs#9291, twbs#9430, twbs#9604, twbs#9686,
twbs#9929, twbs#10731, twbs#10793, twbs#11305, twbs#11498, twbs#11533, twbs#11570, twbs#11604, twbs#11652.

(dem issues, tho)
stempler pushed a commit to stempler/bootstrap that referenced this issue Nov 4, 2014
Original discussion:
less/less.js#1437 (comment).

Since we’re switching to `grunt-contrib-less`, we can take advantage of
newer LESS features than what RECESS supported. Included in that is the
ability to `:extend`, and not only that, but `:extend(.mixin-name
all)`. By doing so, we remove duplicate CSS for all our elements that
were being clearfix-ed.

Fixes twbs#8947, twbs#8968, twbs#8991, twbs#9257, twbs#9268, twbs#9291, twbs#9430, twbs#9604, twbs#9686,
twbs#9929, twbs#10731, twbs#10793, twbs#11305, twbs#11498, twbs#11533, twbs#11570, twbs#11604, twbs#11652.

(dem issues, tho)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants