-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
feat(compiler-sfc): deep selector supports multiple values #8232
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
1eddacf
to
b2bc080
Compare
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: |
WalkthroughThe changes expand the selector rewriting logic in the style scoping plugin to support multiple and nested selectors within special pseudo-selectors like Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant CompilerSFC
participant StylePluginScoped
TestRunner->>CompilerSFC: Compile style block with complex selectors
CompilerSFC->>StylePluginScoped: Call rewriteSelector(selector)
StylePluginScoped->>StylePluginScoped: Process pseudo-selectors (:deep, :slotted, :global)
StylePluginScoped-->>CompilerSFC: Return rewritten selector(s) with scoped attributes
CompilerSFC-->>TestRunner: Output compiled CSS
Estimated code review effort4 (60–120 minutes) Possibly related PRs
Suggested labels
Poem
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/compiler-sfc/src/style/pluginScoped.ts (1)
215-215
: Variable shadowing could cause confusion.The local
nextNodes
variable shadows the outer scope variable declared at line 104. While this appears intentional for local scope management, consider renaming it to avoid potential confusion (e.g.,slottedNextNodes
).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileStyle.spec.ts
(6 hunks)packages/compiler-sfc/src/style/pluginScoped.ts
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test / e2e-test
🔇 Additional comments (7)
packages/compiler-sfc/src/style/pluginScoped.ts (4)
102-104
: Good design for supporting multiple selectors.The introduction of
nodes
array for collecting multiple injection points andnextNodes
for managing subsequent nodes is a clean approach to support multiple selectors within pseudo-selectors.
257-257
: Correct fix for multiple global selectors.The use of spread operator to include all inner nodes is the correct approach for supporting multiple selectors within
:global()
.
297-299
: Proper attribute injection for multiple nodes.The implementation correctly collects all nodes that need attribute injection and applies the scoped attributes to each one. The logic handles both single and multiple injection points appropriately.
Also applies to: 334-346
135-181
: Missing edge-case tests for :deep/::v-deep – please verify.I couldn’t find any existing tests covering empty or malformed
:deep()
/::v-deep()
selectors (including comma-separated edge cases). The selector-cloning logic is complex and warrants explicit coverage.• Add tests for:
–:deep()
and::v-deep()
with no arguments
– Multiple, comma-separated entries (e.g.:deep(.a, .b)
)
– Inputs with leading/trailing whitespace or unexpected syntaxEnsure these scenarios behave as intended and prevent regressions.
packages/compiler-sfc/__tests__/compileStyle.spec.ts (3)
129-201
: Comprehensive test coverage for deep selectors.The test cases thoroughly cover various scenarios for multiple selectors in
:deep
/::v-deep
:
- Simple comma-separated selectors
- Selectors with descendant combinators
- Complex nested structures
- Edge cases with 4+ selectors
Good test design!
209-248
: Good test coverage for slotted selectors.The tests appropriately verify that multiple selectors work correctly with
:slotted
/::v-slotted
, including proper application of the-s
suffix to the scoped attribute.
257-295
: Excellent edge case testing for global selectors.The tests effectively verify that
:global
/::v-global
correctly:
- Supports multiple comma-separated selectors
- Ignores selectors that appear before the global pseudo-selector
- Ignores selectors that appear after the global pseudo-selector
These edge cases are crucial for maintaining the expected behavior of global selectors.
Summary by CodeRabbit
Bug Fixes
::v-deep
,:deep
,::v-slotted
, and::v-global
pseudo-selectors in scoped CSS, ensuring proper application of the scoped attribute in cases with multiple or combined selectors.Tests