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 self can lead to non-semanticly different output #2053

Closed
mgreter opened this issue Apr 27, 2016 · 4 comments
Closed

Extending self can lead to non-semanticly different output #2053

mgreter opened this issue Apr 27, 2016 · 4 comments

Comments

@mgreter
Copy link
Contributor

mgreter commented Apr 27, 2016

.foo[disabled] {
    @extend .foo;
    background: blue;
}
.bar[disabled] {
  foo {
    @extend .bar;
    background: blue;
  }
}
foo {
  .baz[disabled] {
    @extend .baz;
    background: blue;
  }
}

ruby sass:

.foo[disabled] { background: blue; }
.bar[disabled] foo { background: blue; }
foo [disabled].baz { background: blue; }

libsass with #2052

[disabled].foo { background: blue; }
.bar[disabled] foo { background: blue; }
foo [disabled].baz { background: blue; }

//CC @chriseppstein any idea why ruby sass differs in .bar[disabled] vs [disabled].baz?

@chriseppstein
Copy link
Contributor

@mgreter I'm no expert in the extend code. But it seems like this is the reason:
https://github.com/sass/sass/blob/stable/lib/sass/selector/simple_sequence.rb#L193-L195

This is a non-semantic difference in a corner case where the @extend is pointless (it's extending itself). I wouldn't even bother to write a test for this. Just whistle and walk away.

@xzyfer xzyfer modified the milestone: 3.4 May 20, 2016
@xzyfer
Copy link
Contributor

xzyfer commented May 21, 2016

Rescoping this to 3.4.1 since it shouldn't block 3.4.0.

@xzyfer xzyfer modified the milestones: 3.4.1, 3.4 May 21, 2016
@xzyfer xzyfer changed the title Extending of "own" selector can lead to invalid results Extending self can lead to non-semanticly different output May 21, 2016
@xzyfer xzyfer modified the milestones: 3.4.1, 3.4.x Dec 28, 2016
mgreter added a commit to mgreter/sass-spec that referenced this issue Nov 12, 2017
mgreter added a commit to mgreter/sass-spec that referenced this issue Nov 12, 2017
@mgreter
Copy link
Contributor Author

mgreter commented Nov 12, 2017

Input:

.bar.baz {
  @extend .bar;
  background: blue;
}

Expected:

.bar.baz {
  background: blue;
}

Current:

.baz.bar {
  background: blue;
}

Reason:

if modified_original || !replace || groups.empty?
  # First Law of Extend: the result of extending a selector should
  # (almost) always contain the base selector.
  # See https://github.com/nex3/sass/issues/324.
  original = Sequence.new([SimpleSequence.new(members, @subject, source_range)])
  original.add_sources! sources
  groups.unshift original
end
groups.uniq!

mgreter added a commit to mgreter/sass-spec that referenced this issue Nov 12, 2017
@xzyfer
Copy link
Contributor

xzyfer commented Nov 12, 2017

IMHO we shouldn't worry about non-semantically different output anymore. Dart Sass already produces semantically equivalent but different output. In sass-spec we can added libsass specific output with expected_output-libsass.css. I personally don't think implementing this sorting behaviour is worth the performance, and maintainer overhead.

Up to you @mgreter if you think this is worth investing in. I'm happy to close it.

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

No branches or pull requests

3 participants