fix(compiler-sfc): support global selector nesting #12416
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-ssr
@vue/compiler-sfc
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
| expect(compileScoped(`.baz .qux ::v-global(.foo .bar) { color: red; }`)) | ||
| .toMatchInlineSnapshot(` | ||
| ".foo .bar { color: red; | ||
| ".baz .qux[data-v-test] .foo .bar { color: red; |
There was a problem hiding this comment.
The original behavior was that global would ignore the preceding rules, but it will no longer do so after this PR. this will introduce some edge cases. see:
edison1105
added
scope: sfc
🔨 p3-minor-bug
There was a problem hiding this comment.
I've been experimenting with this and I'm wondering about some apparent inconsistencies in how selectors are handled. For example:
The background-color is applied to some of the headings, but not others. It's not really clear to me what would be considered correct here, but I can't really justify the differences.
|
@skirtles-code The intended behavior seems to be that the compiler always scopes the deepest part of the selector. So in But what if the deepest element is global, like in Extending this logic to CSS nesting, it doesn't matter what happens when you use a Side note: I think it would have been nice if Vue 3 were originally designed with only |
WalkthroughThis change updates scoped CSS rewriting to correctly handle ::v-global so that following selectors are scoped appropriately while preserving outer context. It adds a wrapped-global state, propagates it through wrappers, skips attribute injection for globally wrapped rules, and expands tests to cover nested and combination selectors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as SFC Author
participant SFC as SFC Compiler
participant PS as pluginScoped.rewriteSelector
participant Wrap as extractAndWrapNodes
note over Dev,SFC: <style scoped> with selectors using ::v-global(...)
Dev->>SFC: Submit SFC for compilation
SFC->>PS: Rewrite each selector (scoped attrs)
alt Selector contains ::v-global(...)
PS->>PS: Replace ::v-global with its inner nodes
PS->>PS: Determine if resulting selector is a single node
alt Single node (globally wrapped)
PS->>Wrap: extractAndWrapNodes(rule, wrappedGlobal=true)
Wrap-->>PS: Wrapper with __global=true
note right of PS: Mark rule as globally wrapped
PS->>PS: Skip attribute injection (due to __global)
else Multiple nodes
PS->>Wrap: extractAndWrapNodes(rule, wrappedGlobal=false)
Wrap-->>PS: Wrapper with __global=false
PS->>PS: Continue normal injection on non-global parts
end
else No ::v-global
PS->>PS: Perform normal scoped attribute injection
end
PS-->>SFC: Rewritten CSS AST
SFC-->>Dev: Emitted scoped CSS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compiler-sfc/src/style/pluginScoped.ts (1)
241-249: Propagate global-wrapped state into @rules to avoid accidental scoping.When wrapping declarations under nested at‑rules inside a global block, the wrapper created from the at‑rule should inherit
wrappedGlobal. Today it doesn’t, so declarations under@media(or similar) inside::v-global(...) { ... }may get scoped incorrectly.Apply:
- const atruleNodes = rule.nodes.filter(node => node.type === 'atrule') + const atruleNodes = rule.nodes.filter(node => node.type === 'atrule') for (const atnode of atruleNodes) { - extractAndWrapNodes(atnode) + extractAndWrapNodes(atnode, wrappedGlobal) }
🧹 Nitpick comments (3)
packages/compiler-sfc/src/style/pluginScoped.ts (2)
104-111: Global-wrapped early-exit is correct; consider bailing earlier.Instead of checking
__globalon every selector node, short‑circuit beforeselector.each(...)whenrule.__global === trueto avoid needless iteration on wrapper rules like&. Behavior stays identical; it’s a small readability/efficiency win.
292-308: Wrapper flagging looks good; add a brief doc note.
extractAndWrapNodes(..., wrappedGlobal)+(wrappedRule as any).__global = wrappedGlobalis clear. Add a short comment explaining this prevents id injection on the “&” wrapper only, while allowing nested rules (e.g.,h1) to be scoped under a global ancestor.packages/compiler-sfc/__tests__/compileStyle.spec.ts (1)
227-235: Nested global behavior is validated; add @media/combinator cases too.Please add:
::v-global(body){ @media (max-width:600px){ color:red } }(ensure no id on wrapper under @media).:global(body) > h1and.a :global(.b) > .c(combinators).:global(.a, .b) .c(comma list) to lock behavior discussed above.I can draft these test cases if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileStyle.spec.ts(1 hunks)packages/compiler-sfc/src/style/pluginScoped.ts(5 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: continuous-release
🔇 Additional comments (5)
packages/compiler-sfc/src/style/pluginScoped.ts (1)
196-202: Confirm behavior for multiple inner selectors in :global().For
:global(.a, .b),n.replaceWith(...n.nodes)will inject multiple inner selectors. Please confirm the intended scoping result and add a test (comma list + with/without trailing descendants), as selector list flattening here can be tricky in postcss-selector-parser.packages/compiler-sfc/__tests__/compileStyle.spec.ts (4)
203-207: Correct: scope the deepest non-global segment before a trailing global.Expectation
.baz .qux[data-v-test] .foo .bar { ... }matches the intended rule.
209-214: Good::global(body) h1scopesh1while preservingbody.This guards the core regression from #12404.
215-220: Good: pure-global selector remains unscoped.
:global(body h1)compiling tobody h1(no id) is correct.
221-226: Good: preserves outer context before global.
html ::v-global(body) h1→html body h1[data-v-test]is consistent with “scope the right‑most non‑global”.
4 participants
fix #12404
fix #12405
Summary by CodeRabbit
Bug Fixes
Tests