Skip to content
New issue

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

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Extending inner of :has or :not can lead to invalid css #2055

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

Extending inner of :has or :not can lead to invalid css #2055

mgreter opened this issue Apr 27, 2016 · 5 comments
Labels
Bug - Confirmed Bug - @extends Compatibility - P3 Minorly important for compatibility with the Sass spec and ecosystem Dev - Test Written Sass 3.4 Compatibility
Milestone

Comments

@mgreter
Copy link
Contributor

mgreter commented Apr 27, 2016

Closely related to #2054

With the "support" of extending wrapped selectors, we opened a lot of previousely "unreachable" issues. I was pretty aware that these would surface at some point since I know from the ruby code that there are a few special rules regarding wrapped selectors. Probably the most prominent (not sure if there are even others) are :not and :has.

#2052 should fix the most urgent issues, but will not fix this one here:

:has(.thing) {
    color: red;
}
:has(.thing[disabled]) {
    @extend .thing;
    background: blue;
}

Should produce

:has(.thing, :has(.thing[disabled])) {
  color: red; }
:has(.thing[disabled], [disabled]:has(.thing[disabled])) {
  background: blue; }

Note that this is very different from what :not would produce.

Spec sass/sass-spec#915

@mgreter
Copy link
Contributor Author

mgreter commented Apr 27, 2016

@chriseppstein @xzyfer I guess there are a few todo specs for this already in sass-spec, but probably not for very complex cases. To implement this properly (which will not be easy) we pretty sure need a more complete test set, containing tests as the following:

:not(.thing) {
    color: red;
}
:not(.thing[disabled]) {
    @extend .thing;
    background: blue;
}
:has(:not(.thing[disabled])) {
    @extend .thing;
    background: blue;
}

At the moment it is very easy to trip off libsass with anything beside the most basic cases with wrapped selectors. This may even warrant a rewrite of extend, as it currently does not fit the rest of the libsass code base (implementation is a one to one conversion from ruby sass code)

@chriseppstein
Copy link
Contributor

implementation is a one to one conversion from ruby sass code

If it's a 1-1 impl why is it hard to trip it up? Maybe we've been fixing bugs since then and the specs haven't caught up. If you can tell me when the port was done, I can try to write specs for changes to the extend algorithm since that point.

@xzyfer xzyfer modified the milestone: 3.4 May 20, 2016
@xzyfer xzyfer modified the milestones: 3.4.1, 3.4 Sep 8, 2016
xzyfer added a commit to xzyfer/sass-spec that referenced this issue Sep 8, 2016
@xzyfer xzyfer modified the milestones: 3.4.1, 3.4.x Dec 28, 2016
@mgreter mgreter changed the title Extending inner of :has can lead to invalid css Extending inner of :has or :not can lead to invalid css Jan 18, 2017
@mgreter
Copy link
Contributor Author

mgreter commented Jan 18, 2017

@kenjiqq
Copy link

kenjiqq commented Jul 5, 2017

If your project is using a lot of pseudo selectors with common classes like :not(.btn) be careful about extending these classes. One extend of a common button class in our project cased the generated css to go from 1.1M to 2.1M with a lot of invalid css.

@nex3 nex3 added the Compatibility - P3 Minorly important for compatibility with the Sass spec and ecosystem label Jun 3, 2019
@mgreter
Copy link
Contributor Author

mgreter commented Oct 5, 2019

Closing this since extend has been back-ported from dart sass.
Please create new issues in case anything in this regard is still failing.

@mgreter mgreter closed this as completed Oct 5, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug - Confirmed Bug - @extends Compatibility - P3 Minorly important for compatibility with the Sass spec and ecosystem Dev - Test Written Sass 3.4 Compatibility
Projects
None yet
Development

No branches or pull requests

5 participants