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

the behavior of selector.extend does not actually match the extend directive #1301

Closed
stof opened this issue Apr 30, 2021 · 4 comments · Fixed by #1378
Closed

the behavior of selector.extend does not actually match the extend directive #1301

stof opened this issue Apr 30, 2021 · 4 comments · Fixed by #1378
Assignees
Labels

Comments

@stof
Copy link
Contributor

stof commented Apr 30, 2021

@extend forbids extending a compound selector. However, selector.extend happily accepts a:hover as its $extendee argument.
this makes the doc confusing, as it describes the function as following the same rules than @extend: https://sass-lang.com/documentation/modules/selector#extend

Thus, the doc about @extend says that @extend .a:hover should be replaced with @extend .a, :hover as it does the same (and indeed, in libsass, it does the same with a warning). However, passing .a, :hover or .a:hover to selector.extend produces a different behavior than passing .a, :hover to selector.extend.

.hoverlink { @extend .a, :hover }
foo .a:hover, bar .a, b :hover { text-decoration: underline }

f { 
    c: selector-extend("foo .a:hover, bar .a, b :hover", ".a, :hover", ".hoverlink") 
}

Here is the output with dart-sass 1.32.2:

foo .a:hover, foo .hoverlink, bar .a, bar .hoverlink, b :hover, b .hoverlink {
  text-decoration: underline;
}

f {
  c: foo .a:hover, foo .hoverlink, bar .a, b :hover;
}
@stof
Copy link
Contributor Author

stof commented Apr 30, 2021

note that libsass gets the output right when passing .a, :hover to selector-extend (i.e. it handles selector lists of simple selectors properly) but does not produce the same output when passing the compound selector .a:hover

@stof
Copy link
Contributor Author

stof commented Apr 30, 2021

Looking at the code, I found ExtendMode which describes that @extend and selector-extend don't actually run the same logic (they use different modes). So this might be a documentation issue rather than a bug in dart-sass then.

@nex3
Copy link
Contributor

nex3 commented Apr 30, 2021

This looks like a bug to me.

Looking at the code, I found ExtendMode which describes that @extend and selector-extend don't actually run the same logic (they use different modes). So this might be a documentation issue rather than a bug in dart-sass then.

The ExtendMode is different only because all of the target selectors in selector-extend() are required to be used. Otherwise, the semantics should be identical.

@nex3 nex3 added the bug label Apr 30, 2021
@nex3
Copy link
Contributor

nex3 commented Jun 24, 2021

Looking at this further:

  • The current behavior of $extendee: ".a:hover" is intentional. It matches the behavior of LibSass and the historical behavior of Ruby Sass. It's essentially a holdover from allowing compound selectors in @extend, and I'm not sure why we explicitly replicated it in Dart Sass. We should probably deprecate it at some point. I've filed Forbid compound selector extensions in selector functions sass#3089 to track that.

  • The behavior of $extendee: ".a, :hover" is clearly a bug that I'll see if I can go ahead and fix.

The ExtendMode is different only because all of the target selectors in selector-extend() are required to be used. Otherwise, the semantics should be identical.

I was wrong about this—ExtendMode does explicitly enable the behavior you're describing.

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

Successfully merging a pull request may close this issue.

2 participants