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

Extending placeholder selector with nested selector with &-suffix doesn't work #2262

Closed
pepa-linha opened this issue Mar 20, 2017 · 5 comments

Comments

@pepa-linha
Copy link

pepa-linha commented Mar 20, 2017

When using this placeholder selector (compare with mixin - https://gist.github.com/pepa-linha/43ffbce8e60d96e04c7bbe3d22923169)

%component {
  background: red;
  &-sub {
    background: orange;
  }
}

.Component {
    @extend %component;
}

I get this output

.Component {
    background: red;
}

but I would expect this output

.Component {
    background: red;
}

/* This selector is missing */
.Component-sub {
    background: orange;
}
@chriseppstein
Copy link

You're expectation is incorrect. Placeholders are a kind of selector and they work exactly like classes. You defined two different placeholders and you only extended one of them.

Mixins may work better for you in this case, or you'll need to extend both placeholders:

.Component {
    @extend %component;
    &-sub { @extend %component-sub; }
}

Alternatively, if you used two classes this would work fine:

%component {
  background: red;
  &.sub {
    background: orange;
  }
}

.Component {
    @extend %component;
}

And then of course your markup would need to use class="Component sub" instead of class="Component-sub".

@thany
Copy link

thany commented Apr 5, 2017

So it's not possible to make an @extend-able component that has a child/descendant selector in it that uses the & placeholder, and that placeholder isn't entirely its own selector (i.e. it may not be part of a class).

Feels like a strange arbitrary limitation to me. Even though the expectation of @pepa-linha is, as you call it, incorrect, this behaviour still looks like a bug, and I believe is entirely fixable. Just call it a feature request if you must believe this behaviour is by design.

@chriseppstein
Copy link

@thany First and foremost, you need to understand that the code that handles placeholders is the exact same code that handles all selectors because placeholders are a type of selector. Secondly, we have to consider use cases that are not built around component definitions using well defined naming systems like BEM, because the use of & and @extend are generic. So the feature request isn't about placeholders, it's actually about all selectors so what we have to ask is whether we expect the following to work as indicated:

#header {
   &-nav { } // #header-nav
}

#footer {
  @extend #header; // should this produce #footer-nav ? We have decided that it should not.
}

One of the reasons for this decision is that we expect @extend to have the exact same behavior across a nesting refactoring. That is, if you had originally written the code:

#header {}
#header-nav {}
#footer { @extend #header; }

and then later you refactored it to:

#header {
   &-nav { }
}
#footer { @extend #header; }

You should expect the exact same output. This "feature request" would violate this invariant and hence it would actually introduce a bug from another user's perspective and use case.

Finally, It seems like some people have a mental model that @extend applies before resolving the parent selector, that it is like a search and replace. This is fundamentally a wrong way of thinking about @extend which creates a document level styling relationship between complex css selectors and can only be applied to full selectors and not to just parts of them.

@pepa-linha
Copy link
Author

Thank you so much for your explanation. I understand "problem" now. This behavior is much better than in Stylus (which I used before) because Stylus generates different output for last example with #header, #header-nav and #footer. I have to change my thinking 👍

@chriseppstein
Copy link

@pepa-linha the stylus behavior isn't necessarily worse, it's just a different concept (using the same syntax 🤔) -- as long as the concepts are understood, people will know what to expect and behave accordingly. Their model is a simpler one, it's easier to implement and arguably it's easier to understand and know what output it will generate -- but it lacks the document level guarantees that Sass makes 🤷‍♂️

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

3 participants