fix(compiler-sfc): resolve :deep inside :is/:where at start position#14736
fix(compiler-sfc): resolve :deep inside :is/:where at start position#14736ZackaryShen wants to merge 1 commit intovuejs:mainfrom
Conversation
Fix issue vuejs#14724 where :deep at the start of :is/:where selector was not being properly resolved to [data-v-xxx]. Before: :is(:deep(.foo)) -> :is(:deep(.foo)) After: :is(:deep(.foo)) -> :is([data-v-xxx] .foo) The fix checks if __deep was set during recursion and injects the scoped attribute at the start of the inner selector.
📝 WalkthroughWalkthroughThis PR fixes scoped CSS compilation for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/compiler-sfc/src/style/pluginScoped.ts`:
- Around line 249-268: The new injection block incorrectly calls
selector.insertBefore with an inner-selector node (causing wrong placement and
duplicate scoped attributes) and it’s also skipped for cases like
:is(:deep(.foo)) .bar because rewriteSelector’s node-tracking lets later
siblings overwrite the recorded pseudo; fix by removing this explicit insertion
(rely on the recursive call in rewriteSelector to scope the inner :deep/:deep()
content) or, if you prefer explicit handling, perform insertions on the inner
Selector container (use (node as selectorParser.Pseudo).nodes[0] as the
container) rather than the outer selector and only when shouldInject is false to
avoid double-injection; additionally update rewriteSelector’s node-tracking so
the recorded node (the :is/:where pseudo) is not clobbered by subsequent sibling
nodes so the :is branch still runs for selectors like :is(:deep(.foo)) .bar.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a686998f-d288-429f-abe6-2ca1631da6f0
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileStyle.spec.tspackages/compiler-sfc/src/style/pluginScoped.ts
| // If __deep was set inside :is/:where, we need to inject [id] at the start | ||
| // of the inner selector (before the first node), not after. | ||
| // :is(:deep(.foo)) should become :is([data-v-xxx] .foo) | ||
| if ((rule as any).__deep && (node as selectorParser.Pseudo).nodes.length) { | ||
| selector.insertBefore( | ||
| (node as selectorParser.Pseudo).nodes[0], | ||
| selectorParser.attribute({ | ||
| attribute: slotted ? id + '-s' : id, | ||
| value: slotted ? id + '-s' : id, | ||
| raws: {}, | ||
| quoteMark: `"`, | ||
| }), | ||
| ) | ||
| selector.insertBefore( | ||
| (node as selectorParser.Pseudo).nodes[0], | ||
| selectorParser.combinator({ | ||
| value: ' ', | ||
| }), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Injection targets the wrong container and double‑injects the scoped attribute.
Three concerns with this block:
-
Wrong insertion target.
selectorhere is the outer selector that contains the:is(...)pseudo (node).(node as Pseudo).nodes[0]is the first innerSelectorof:is, which is a child ofnode, not ofselector.Container.insertBefore(oldNode, newNode)inpostcss-selector-parserresolves the position viathis.index(oldNode); whenoldNodeis not a direct child,indexreturns-1andArray.prototype.splice(-1, 0, newNode)inserts near the end of the outer selector — not inside the inner selector as intended. -
Double injection. The preceding
forEachalready recurses into each inner selector with the caller'sdeep(false at the top level), so inside the recursionshouldInject = !deepistrue. When the recursion processes:deep(.foo)it rewrites it to[' ', '.foo'], then the existingif (shouldInject)block (lines 282–295) prepends[data-v-xxx], producing[[data-v-xxx], ' ', .foo]. The inner selector is already correctly scoped — adding another insertion here duplicates it. -
Won’t fire for
:is(:deep(.foo)) .bar. The node‑tracking loop (lines 221–228) only assigns:is/:wheretonodewhen!node. With a trailing class like.bar, that class overridesnode(first clause of the||). So at line 243nodeis.bar, not:is, and neither the recursion at 246–248 nor this new block is reached — meaning the third new test case cannot produce the asserted":is([data-v-test] .foo) .bar"snapshot.
Sketch of a fix that targets the correct container (relying on the recursive call to inject for the :deep part is the simplest option; if you want explicit handling, operate on the inner Selector):
🔧 Proposed direction
- ;(node as selectorParser.Pseudo).nodes.forEach(value =>
- rewriteSelector(id, rule, value, selectorRoot, deep, slotted),
- )
- // If __deep was set inside :is/:where, we need to inject [id] at the start
- // of the inner selector (before the first node), not after.
- // :is(:deep(.foo)) should become :is([data-v-xxx] .foo)
- if ((rule as any).__deep && (node as selectorParser.Pseudo).nodes.length) {
- selector.insertBefore(
- (node as selectorParser.Pseudo).nodes[0],
- selectorParser.attribute({
- attribute: slotted ? id + '-s' : id,
- value: slotted ? id + '-s' : id,
- raws: {},
- quoteMark: `"`,
- }),
- )
- selector.insertBefore(
- (node as selectorParser.Pseudo).nodes[0],
- selectorParser.combinator({
- value: ' ',
- }),
- )
- }
- shouldInject = false
+ ;(node as selectorParser.Pseudo).nodes.forEach(inner =>
+ rewriteSelector(id, rule, inner, selectorRoot, deep, slotted),
+ )
+ // The recursive call above already injects [id] at the start of the
+ // inner selector when :deep is encountered there, so nothing extra
+ // needs to be done on the outer selector here.
+ shouldInject = falseIf a trailing combinator (e.g. :is(:deep(.foo)) .bar) also needs to be handled, the :is/:where tracking at lines 221–228 has to be extended so that node still points at the pseudo even when later siblings exist — otherwise this whole branch is skipped.
Please run the three new snapshot cases and confirm the actual output; I suspect at least the third (:is(:deep(.foo)) .bar) will not match the recorded snapshot and the first two may emit a stray scoped attribute outside :is(...).
#!/bin/bash
# Verify that the new test cases in compileStyle.spec.ts actually pass.
fd -t f 'compileStyle.spec.ts' packages/compiler-sfc
rg -n -C2 ':is\(:deep|:where\(:deep' packages/compiler-sfc/__tests__/compileStyle.spec.ts
# And inspect rewriteSelector’s node-tracking logic to confirm the :is path is reached for the `:is(:deep(.foo)) .bar` case.
ast-grep --pattern $'function rewriteSelector($$$) { $$$ }'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/compiler-sfc/src/style/pluginScoped.ts` around lines 249 - 268, The
new injection block incorrectly calls selector.insertBefore with an
inner-selector node (causing wrong placement and duplicate scoped attributes)
and it’s also skipped for cases like :is(:deep(.foo)) .bar because
rewriteSelector’s node-tracking lets later siblings overwrite the recorded
pseudo; fix by removing this explicit insertion (rely on the recursive call in
rewriteSelector to scope the inner :deep/:deep() content) or, if you prefer
explicit handling, perform insertions on the inner Selector container (use (node
as selectorParser.Pseudo).nodes[0] as the container) rather than the outer
selector and only when shouldInject is false to avoid double-injection;
additionally update rewriteSelector’s node-tracking so the recorded node (the
:is/:where pseudo) is not clobbered by subsequent sibling nodes so the :is
branch still runs for selectors like :is(:deep(.foo)) .bar.
|
duplicate of #14725 |
2 participants
Description
Fix issue #14724 where
:deepat the start of:is/:whereselector was not being properly resolved.Before
Was compiled to:
After
Is compiled to:
Test cases added
:is(:deep(.foo)):where(:deep(.foo)):is(:deep(.foo)) .barFixes #14724
Summary by CodeRabbit
Bug Fixes
:deep()is used within:is()and:where()pseudo-classes, ensuring proper selector rewriting.Tests
:deep()in:is()and:where()pseudo-selectors.