-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
fix(compiler-sfc): improve scoped style rewriting for nested and non-pseudo :is/:where selectors #12244
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
base: main
Are you sure you want to change the base?
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Maybe it's another bug that I reproduced on the current official playground I created a new issue to track it #12252 |
According to the document, I believe the behavior with this PR is proper. |
This pr is correct, you are so cool @edison1105 |
I'm wondering about nesting A selector like |
WalkthroughThe changes update the logic for handling scoped CSS selectors, specifically improving how nested Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant Compiler
participant SelectorRewriter
TestSuite->>Compiler: Compile SFC with scoped CSS
Compiler->>SelectorRewriter: Rewrite selectors for scoping
SelectorRewriter->>SelectorRewriter: Check for :is()/:where() (including nesting)
SelectorRewriter->>SelectorRewriter: Inject scoped attribute as needed
SelectorRewriter-->>Compiler: Return rewritten selectors
Compiler-->>TestSuite: Output compiled CSS
Assessment against linked issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@skirtles-code |
I think we may need to take a step back and consider a more comprehensive fix. It seems we’re not correctly identifying subject selectors in general when they don’t include element, class, or attribute selectors. The following input: <style scoped>
:hover {
color: red;
}
.main ~ :hover {
color: red;
}
.main + :hover {
color: red;
}
.main ::before {
color: red;
}
.main :is(.title) {
color: red;
}
</style> is compiled to [data-v-7ba5bd90]:hover {
color: red;
}
.main[data-v-7ba5bd90] ~ :hover {
color: red;
}
.main[data-v-7ba5bd90] + :hover {
color: red;
}
.main[data-v-7ba5bd90] ::before {
color: red;
}
.main[data-v-7ba5bd90] :is(.title) {
color: red;
} while it should be: [data-v-7ba5bd90]:hover {
color: red;
}
.main ~ [data-v-7ba5bd90]:hover {
color: red;
}
.main + [data-v-7ba5bd90]:hover {
color: red;
}
.main [data-v-7ba5bd90]::before {
color: red;
}
.main [data-v-7ba5bd90]:is(.title) {
color: red;
} |
@Justineo You may well be right. I did wonder about that when reviewing this PR, but I thought it was a small step in the right direction, which might prove easier than trying to make a big leap. While testing this PR, I avoided testing with other pseudos, because they're known to be broken: #8868. I didn't want to increase the scope of this PR unnecessarily. Different pseudos need different handling. For With hindsight, maybe I was missing a more important point. Maybe we already have all the special casing we need for |
Why do we need to inject attribute selectors inside the arguments of |
@Justineo Good question. I spent some time going through the history of this. I think the most enlightening was #8929. Long story short, it's for The motivation was Tailwind's use of In Vue 3.5.16 (and other recent versions) we compile this: :is(a b) {}
:is(a :deep(b)) {} to this: :is(a b[data-v-7ba5bd90]) {}
:is(a[data-v-7ba5bd90] b) {} The first line could compile to It isn't consistent, though. There are lots of edge cases, some of which are fixed by this PR, some of which aren't. e.g. Getting the specificity right when combining I wonder whether the |
Hmm… this is turning out to be more complex than I expected — especially when combining Vue’s selector extensions ( I’ve put together a (very incomplete) table that only includes
*: for minimal specificity impact I hope there’s a way to establish clear guidelines that can help ensure we implement scoped styles as accurately as possible. For reference, here's the compiled output of the above code by Svelte (Svelte only has a
|
close #12241
Summary by CodeRabbit
Bug Fixes
:is()
and:where()
pseudo-classes, ensuring the scoped attribute is correctly applied in complex selector scenarios.Tests
:is()
and:where()
selectors with ID prefixes.