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

Loosen :is() wrapping rules in applyImportantSelector for more readable output #13900

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

joshwilsonvu
Copy link

@joshwilsonvu joshwilsonvu commented Jun 26, 2024

Hi, this is just a small quality of life improvement to the changes made in #10835, intended for a v3.x.

I'm using the important selector strategy, and noticed that for most cases, output like

.tw :is(.text-citrine-500) {
  --tw-text-opacity: 1;
  color: rgb(246 220 82 / var(--tw-text-opacity)) /* #f6dc52 */;
}

can be reduced to simply

.tw .text-citrine-500 {
  --tw-text-opacity: 1;
  color: rgb(246 220 82 / var(--tw-text-opacity)) /* #f6dc52 */;
}

because the nested selector doesn't contain a descendant selector like .dark .text-citrine-500. I'm sure the unnecessary :is() would be removed in minification, but having the "cleanest" possible output for Intellisense would be nice. (Trying to convince my coworkers to get on board, and making the previews feel more "hand-written" might help! 😁)

As far as I can tell, this change can't affect the behavior of any possible selector passed in. Feel free to close if you can find a counterexample. Thank you!

Edit: we're switching from the important selector strategy to prefixing, so we won't actually need this, if that affects whether you'll consider it.

@thecrypticace thecrypticace self-assigned this Jul 16, 2024
@thecrypticace thecrypticace merged commit 0573c07 into tailwindlabs:master Jul 16, 2024
13 checks passed
@dickeylth
Copy link

This solves a critical performance issue in our long list scroll render scene, thanks! Great job!

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 this pull request may close these issues.

3 participants