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

Nested SCSS selectors breaking @apply directive #9742

Closed
shengslogar opened this issue Nov 4, 2022 · 10 comments · Fixed by #9765
Closed

Nested SCSS selectors breaking @apply directive #9742

shengslogar opened this issue Nov 4, 2022 · 10 comments · Fixed by #9765

Comments

@shengslogar
Copy link

What version of Tailwind CSS are you using?

v3.2.1

What build tool (or framework if it abstracts the build tool) are you using?

Laravel Mix v6.0.49

What version of Node.js are you using?

v16.18.0

What browser are you using?

N/A

What operating system are you using?

macOS Ventura

Reproduction URL

https://github.com/shengslogar/tailwind-scss-nesting-bug-report/blob/main/output.css#L469-L493

Describe your issue

Complex nested SCSS selectors seem to be breaking the @apply directive.

Input:

.foo {
  &:not(.bar) {
    @apply bg-red-100;
    background: red;

    &.baz > .bam {
      @apply bg-orange-100;
      background: orange;
    }
  }
}

Output:

.foo:not(.bar) {
  --tw-bg-opacity: 1;
  background-color: rgb(254 226 226 / var(--tw-bg-opacity));
  background: red;
}

.foo.baz.bam:not(.bar) >  {
  --tw-bg-opacity: 1;
  background-color: rgb(255 237 213 / var(--tw-bg-opacity));
}

.foo:not(.bar).baz > .bam {
  background: orange;
}

Expected Output:

.foo:not(.bar) {
  --tw-bg-opacity: 1;
  background-color: rgb(254 226 226 / var(--tw-bg-opacity));
  background: red;
}

.foo:not(.bar).baz > .bam {
  --tw-bg-opacity: 1;
  background-color: rgb(255 237 213 / var(--tw-bg-opacity));
  background: orange;
}

Thanks in advance! 🙏

@adamwathan
Copy link
Member

This seems to generate the wrong CSS even without Sass or nesting at all, so just dropping this reproduction here since it might help us trace it down faster:

https://play.tailwindcss.com/a12qnV4sVd?file=css

Thanks for reporting!

@adamwathan
Copy link
Member

I was expecting this might be the same bug as this one: #9668

...but it still seems to behave incorrectly even on our insiders build, so will look at it next week 👍

@adamwathan
Copy link
Member

Actually it is generating better output in insiders — output that is correct but could be minified better:

/* v3.2.1 */
.foo:not(.bar) {
  --tw-bg-opacity: 1;
  background-color: rgb(254 226 226 / var(--tw-bg-opacity));
  background: red;
}

.foo.baz.bam:not(.bar) >  {
  --tw-bg-opacity: 1;
  background-color: rgb(255 237 213 / var(--tw-bg-opacity));
}

.foo:not(.bar).baz > .bam {
  background: orange;
}

/* Insiders */
.foo:not(.bar) {
  --tw-bg-opacity: 1;
  background-color: rgb(254 226 226 / var(--tw-bg-opacity));
  background: red;
}

.foo.baz:not(.bar) > .bam {
  --tw-bg-opacity: 1;
  background-color: rgb(255 237 213 / var(--tw-bg-opacity));
}

.foo:not(.bar).baz > .bam {
  background: orange;
}

The last two rules should be able to be collapsed so will leave this open as a reminder to look at that, but the behavior should at least be correct in the insiders build.

@adamwathan
Copy link
Member

Here's a minimal reproduction for the lack of collapsing:

https://play.tailwindcss.com/XtHxx2TRfT?file=css

image

My guess is because we are always putting pseudo-classes at the end of any selector "group", which I think is technically necessary for pseudo-classes like :hover but not for pseudo-classes that have braces like :not(...).

@shengslogar
Copy link
Author

Awesome, makes sense. Appreciate the quick response and investigation! What's the process for insiders making it upstream to a tagged release? Thanks for all you do, Adam 🙏

@adamwathan
Copy link
Member

We'll tag a proper release probably on Monday! In the mean time can just npm install tailwindcss@insiders to use the insiders build.

@shengslogar
Copy link
Author

Oh, beautiful, didn't check NPM. I was looking for a GitHub tag and only found a single outdated one 👍

@adamwathan
Copy link
Member

Just tagged a real release for you too 👍

@shengslogar
Copy link
Author

My man

@shengslogar
Copy link
Author

Yep, that did the trick! Much appreciated. Feel free to close (or rename) to address that other collapsing issue. Thanks!!

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

Successfully merging a pull request may close this issue.

2 participants