Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Failure to optimize away unnecessary selectors #511

Closed
nex3 opened this Issue Sep 10, 2012 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

nex3 commented Sep 10, 2012

%nav {
  width: 100%;

  .brand {
    color: #fff;
  }

  .btn {
    background: blue;
  }
}

.myNav {
  @extend %nav;

  .button {
    @extend .btn;
  }
}

.subnav {
  @extend %nav;
}

compiles to

.myNav, .subNav {
  width: 100%;
}
.myNav .brand, .subNav .brand {
  color: #fff;
}
.myNav .btn, .subNav .btn, .myNav .button, .subNav .myNav .button, .myNav .subNav .button {
  background: blue;
}

but should compile to

.myNav, .subNav {
  width: 100%;
}
.myNav .brand, .subNav .brand {
  color: #fff;
}
.myNav .btn, .subNav .btn, .myNav .button {
  background: blue;
}

.subNav .myNav .button and .myNav .subNav .button are subselectors of .myNav .button and should get optimized away.

A not quite the same example, but relevant (I think) to this issue is general scoping for placeholders. I’ve put together a gist here: https://gist.github.com/makenosound/5079966.

I can see that you’d want to have common properties collated in the redundant rule, but ideally the compiler would be smart enough to ignore properties that get overridden in the nested @extend.

Contributor

nex3 commented Mar 4, 2013

That's a lot harder of an optimization to do than removing redundant selectors in the same rule. Sass not only needs to figure out that the two rules have the same selector, but that the intervening rule can't interfere with the first and last selector. This is something we'd like to do at some point, but it's out-of-scope for this issue.

Make sense, ta for weighing in. I’ll just have to be content with being explicit with my placeholder classes for now :)

@nex3 nex3 closed this in 3700884 Mar 22, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment