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

Prevent not-* from being used with variants with multiple sibling rules #15689

Merged
merged 5 commits into from
Jan 21, 2025

Conversation

thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Jan 21, 2025

Variants like this can't be easily negated by our current system:

@custom-variant dark {
  &.is-dark {
    @slot;
  }
  @media (prefers-color-scheme: dark) {
    @slot;
  }
}

Right now it produces the following CSS which is logically incorrect:

.utility-name {
  &:not(.is-dark) {
    /* ... */
  }

  @media not (prefers-color-scheme: dark) {
    /* ... */
  }
}

The correct CSS is this which requires moving things around:

.utility-name {
  @media not (prefers-color-scheme: dark) {
    &:not(.is-dark) {
      /* ... */
    }
  }
}

We're opting to disable this instead of generating incorrect CSS for now. I'd like to bring this back in the future for simpler cases in the future.

@thecrypticace thecrypticace requested a review from a team as a code owner January 21, 2025 15:15
@thecrypticace
Copy link
Contributor Author

@RobinMalfait we should probably merge your custom-variant PR first.

@thecrypticace thecrypticace force-pushed the fix/v4-no-not-siblings branch from 87ecb18 to dee4cfc Compare January 21, 2025 15:22
@thecrypticace thecrypticace changed the title Don’t allow not-* to be used with variants that have siblings Prevent not-* from being used with variants with multiple sibling rules Jan 21, 2025
@thecrypticace thecrypticace merged commit 79f21a8 into next Jan 21, 2025
5 checks passed
@thecrypticace thecrypticace deleted the fix/v4-no-not-siblings branch January 21, 2025 15:38
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.

2 participants