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

Sibling selectors of components don't work on classes applying them #9093

Closed
kerberjg opened this issue Aug 13, 2022 · 4 comments · Fixed by #9107
Closed

Sibling selectors of components don't work on classes applying them #9093

kerberjg opened this issue Aug 13, 2022 · 4 comments · Fixed by #9107
Assignees

Comments

@kerberjg
Copy link

What version of Tailwind CSS are you using?

For example: v3.1.8

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

Tailwind Play

What browser are you using?

Chrome

What operating system are you using?

macOS

Reproduction URL

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

Describe your issue

When applying custom components to selectors with a sibling operator, the output CSS does not apply to them properly. For example, with the following code

@layer components {
    .row.row-odd {
        @apply bg-green-400;
    }
    .row.row-even {
        @apply bg-red-400;
    }
    .row-odd + .row-even {
        @apply bg-blue-400;
    }
    .row-even + .row-odd {
        @apply bg-yellow-400;
    }
}

section:nth-of-type(odd) { @apply row row-odd; }
section:nth-of-type(even) { @apply row row-even; }

...should output something similar to:

section:nth-of-type(odd) + section:nth-of-type(even) { background: blue; }
section:nth-of-type(even) + section:nth-of-type(odd) { background: yellow; }

...and yet it only outputs:

section:nth-of-type(odd) + .row-even { background: blue; }
.row-odd + .section:nth-of-type(even) { background: blue; }
section:nth-of-type(even) + .row-odd { background: yellow; }
.row-even + section:nth-of-type(odd) { background: yellow; }

It also seems that when declaring composite rules (multiple classes, such as .row.row-odd) this breaks during compilation when applying it to a tag instead of a class, resulting in rules such as .rowsection (with no separator, which makes no sense) as a result of the following code:

@layer components {
    .row.row-odd {
        @apply bg-green-400;
    }
}

section {
    @apply row-odd;
}

Both behaviors are observable in the output of the Tailwind Play document I linked

@thecrypticace
Copy link
Contributor

Hey thanks for the report! So there's a few of things here:

  1. We definitely had issues with tags in @apply selectors — thank you for reporting that! Finally gave me a really good reason to redo the selector rewriting code there. This has been fixed by Fix @apply selector rewriting when multiple classes are involved #9107.
  2. Likewise we also had issues replacing multiple occurrences of the same class in a given selector — this has also been fixed by Fix @apply selector rewriting when multiple classes are involved #9107
  3. There is a slight misunderstanding about how @apply works. In general it's meant to work in isolation, on a per-utility basis, and act the same as applying the individual classes to an element.

Each class in @apply class1 class2 is treated separately because of this, as if you wrote @apply class1; @apply class2;

We then copy each of the appropriate rules over for each matching class and replace the "candidate" portion of the selector.

For instance with section:nth-of-type(odd) { @apply row row-odd; } we match against row first, copy it's rules, replace all instances of .row, etc... then match against row-odd and do the same.

As such the expected output is something like this:

/* Rules for section:nth-of-type(odd) */
/* Copy of rule .row.row-odd — added because @apply row */
section.row-odd:nth-of-type(odd) {
  background: green;
}

/* Copy of rule .row.row-odd — added because @apply row-odd */
section.row:nth-of-type(odd) {
  background: green;
}

/* Copy of rule .row.row-even — added because @apply row */
section.row-even:nth-of-type(odd) {
  background: red;
}

/* Copy of rule .row-odd + .row-even — added because @apply row-odd */
section:nth-of-type(odd) + .row-even {
  background: blue;
}

/* Copy of rule .row-even + .row-odd — added because @apply row-odd */
.row-even + section:nth-of-type(odd) {
  background: yellow;
}

/* Rules for section:nth-of-type(even) */
/* Copy of rule .row.row-odd — added because @apply row */
section.row-odd:nth-of-type(even) {
  background: green;
}

/* Copy of rule .row.row-even — added because @apply row */
section.row-even:nth-of-type(even) {
  background: red;
}

/* Copy of rule .row.row-even — added because @apply row-even */
section.row:nth-of-type(even) {
  background: red;
}

/* Copy of rule .row-odd + .row-even — added because @apply row-even */
.row-odd + section:nth-of-type(even) {
  background: blue;
}

/* Copy of rule .row-even + .row-odd — added because @apply row-even */
section:nth-of-type(even) + .row-odd {
  background: yellow;
}

Having said that having section:nth-of-type(odd) { @apply row row-odd; } producing two rules for .row.row-odd is unfortunate but making @apply consider multiple, joined classes at one time is a huge undertaking and not likely to be something we'll tackle any time soon — if at all.

We have a couple of in-depth comments about how @apply works that you can look over if you're interested:

  1. Styling class and then using variant on that class leaks output #6451 (comment)
  2. @apply with peer variants does not work when the selector contains multiple levels #6062 (comment)

@kerberjg
Copy link
Author

Thank you for the quick response and detailed explanation!
I'm not sure I understand fully, however; my issue isn't with too many rules being produced, but rather not enough. It's fine if this happens:

.row-odd + .row-even ...
.row-odd + section:nth-of-type(even) ...

...but I'd like to see:

.row-odd + .row-even ...
.row-odd + section:nth-of-type(even) ...
section:nth-of-type(odd) + section:nth-of-type(even) ...

I probably shouldn't have included .row in my example, my question is more about applying just one of the two classes at once. Basically, if I manually place the utility classes on HTML elements everything works fine, however when I apply them on a tag or another class, the sibling selector is broken.
Here's an improved example:
https://play.tailwindcss.com/cOwEnnbS23

Basically, if I were to write it as a test it'd be closer to:

it('can produce selectors that replace multiple instances of the same class', async () => {
  let config = {
    content: [{ raw: html`<div class="foo-1 -foo-1 new-class"></div>` }],
    plugins: [],
  }

  let input = css`
    .foo + .bar {
      color: blue;
    }
    .bar + .foo {
      color: fuchsia;
    }
    header {
      @apply foo;
    }
    main {
      @apply foo bar;
    }
    footer {
      @apply bar;
    }
  `

  let result = await run(input, config)
  console.log(result.css)

  expect(result.css).toMatchFormattedCss(css`
    .foo + .bar {
      color: blue;
    }
    .bar + .foo {
      color: fuchsia;
    }
    header + footer {
      color: blue;
    }
    main + main {
      color: blue;
      color: fuchsia;
    }
    footer + header {
      color: fuchsia;
    }
  `)
})

BTW, when I run the above code it seems to get stuck in a loop because of main { @apply foo bar; }

If what you said still applies, please let me know

@thecrypticace
Copy link
Contributor

thecrypticace commented Aug 15, 2022

Ah yeah so @adamwathan and I just discussed this regarding my change targeting the same "candidate" in a selector and realized that it's not 100% accurate either so we're backing that bit out currently.

Given:

.foo + .foo { color: red; }
section { @apply foo; }

We would need to end up with four rules for it to be correct:

.foo + .foo { color: red; }
section + .foo { color: red; }
.foo + section { color: red; }
section + section { color: red; }

As it turns out, for any number of classes, we'd end up with approximately 2^N rules that would have to be generated. For example the selector .foo + .foo + .foo + .foo would result in 15 additional rules per use of @apply foo.

For your case specifically we'd also have to maintain a graph of how all the classes interact and are applied with one another. This would mean that earlier rules affect later ones and getting this right generally would be fairly difficult as the cascade — for the same specificity — last declaration wins. I believe in your case the correct generated code would be the following:

.row-odd + .row-even { … }
section:nth-of-type(odd) + .row-even { … }
.row-odd + section:nth-of-type(even) { … }
section:nth-of-type(odd) + section:nth-of-type(even) { … }

And, if you have any additional rules that target .row-odd or .row-even we're now adding 4 more rules for each of those.

All in all — it is entirely possible this is out-of-scope for @apply because of the complexity involved. Adam and I are going to discuss this further.

@kerberjg
Copy link
Author

That sounds about right! Thing is, I understand this generates a lot of output rules, but the only reason someone would do this is if they really needed it.
I've come to this only because I have to work with a frontend framework which doesn't let me specify classes on the enclosing elements when rendering a page (believe me, other people have tried); I only have control over their contents and the tag or ID -based CSS rules.

I believe in your case the correct generated code would be the following

That looks about correct! However... I'm probably oversimplifying, but couldn't they be merged with a comma selector?

if you have any additional rules that target .row-odd or .row-even we're now adding 4 more rules for each of those

If I add a rule to .row-even, wouldn't it end up only in .row-even and section:nth-of-type(even)? Do I need to also have them in the sibling operators? If there were any specificity-caused overrides in the sibling selectors, wouldn't that be simply correct?

it is entirely possible this is out-of-scope for @apply...

Is it? Doesn't the current output feel 'odd' given my second TW Play example?

...because of the complexity involved

That makes sense. Please let me know if I can help somehow

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